Suddenly i get this error message when running gulp sass

  Replace Autoprefixer browsers option to Browserslist config.
  Use browserslist key in package.json or .browserslistrc file.

The browsers: option from gulp-options.yml can be copied over to package.json or .browserslistrc 1:1.

I would go for the .browserslistrc file as it looks cleaner to me. It's also does not belong to package.json imo.

In the package.json file it would look like this (having comments):

"browserslist": [
    "last 2 version",
    "ie >= 11",
    "Safari >= 8",
    "not bb < 10"
  ],
  "browserslist_comments": [
    "not bb < 10 means Support only latest Blackberry 10."
  ]

Also installation needs to copy over the new file and an update hook needs to be written to reflect the changes down the line.

CommentFileSizeAuthor
#12 3066987-replace_autoprefixer_browsers-12.patch4.08 KBpivica
#12 interdiff-11-12.txt3.07 KBpivica
#11 3066987-replace_autoprefixer_browsers-11.patch2.88 KBytsurk
#10 3066987-replace_autoprefixer_browsers-10.patch2.93 KBytsurk
#5 3066987-replace_autoprefixer_browsers-5.patch2.65 KBytsurk

Comments

ytsurk created an issue. See original summary.

ytsurk’s picture

ytsurk
Location Switzerland
commented
Issue summary: View changes
pivica’s picture

pivica commented
Status: Needs review » Needs work

+1 for using .browserslistrc instead of package.json.

ytsurk’s picture

ytsurk
Location Switzerland
commented
Issue summary: View changes
ytsurk’s picture

ytsurk
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new 3066987-replace_autoprefixer_browsers-5.patch2.65 KB

Here the complete patch :D

ytsurk’s picture

ytsurk
Location Switzerland
commented
Title: Replace Autoprefixer browsers option to Browserslist config » Move Autoprefixer browsers gulp-option to .browserslistrc file
pivica’s picture

pivica commented
Issue tags: +next-release
pivica’s picture

pivica commented
Status: Needs review » Needs work 
  1. +++ b/bs_base.drush.inc
@@ -1325,6 +1326,10 @@ EOD;
+    case '.browserslistrc':
+      $content .= "# Browsers that we support\n\nlast 2 version\nie >= 11\nSafari >= 8\nnot bb < 10 # Means Support only latest Blackberry 10.\n";
+      break;

    I don't think we need this here. We will copy .browserlistrc from parent theme in _bs_base_copy_theme_files() call. 

  2. +++ b/themes/bs_bootstrap/bs_bootstrap.bs_base.install
@@ -62,3 +62,19 @@ function bs_bootstrap_bs_update_8001() {
+function bs_bootstrap_bs_update_8002() {

    Shouldn't we also edit gulp-options.yml in $child_themes and remove autoprefixer.browsers option from there?

    It is possible that plugin will complain again if it gets this option? 

  3. +++ b/themes/bs_bootstrap/bs_bootstrap.bs_base.install
@@ -62,3 +62,19 @@ function bs_bootstrap_bs_update_8001() {
+    _bs_base_generate_file('.browserslistrc', $options);

    Ah now i see why you added .browserslistsrc support in _bs_base_generate_sass_file().

    But i think we can just use _bs_base_copy_file() call here and copy .browserslistrc from parent theme. This will prevent us then adding new code to generator. What do you think?

pivica’s picture

pivica commented

If there is time maybe open a new section 'SASS Options' in https://www.drupal.org/docs/8/themes/bs-base and start documenting this options. We can start with autoprefixer and .browserslistrc support for browser versions.

ytsurk’s picture

ytsurk
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new 3066987-replace_autoprefixer_browsers-10.patch2.93 KB

pivica, thanks for your review.

1. I removed the file creation, and just copy it over from bs_bootstrap.Both, bs_base and bs_bootstrap, need the .browsersrc, to allow sass to run for each of them.

2. browsers where only defined in bs_base gulp-options.yml and inherited to the child themes. So no need to remove.

3. I use now copy there, see 1.

When testing the install hook with a custom theme having another custom theme as base theme, both got updated. The theme name ($target_machine_name) needs to be passed to the update_functions too. We missed that in the first two updates. And I created the following follow up issue: #3067574: Make sure update functions only take care of the chosen sub-theme

I also started the documentation here: https://drupal.org/docs/8/themes/bs-base/sass-options

ytsurk’s picture

ytsurk
Location Switzerland
commented
StatusFileSize
new 3066987-replace_autoprefixer_browsers-11.patch2.88 KB

Removed unnecessary loop.

pivica’s picture

pivica commented
StatusFileSize
new interdiff-11-12.txt3.07 KB
new 3066987-replace_autoprefixer_browsers-12.patch4.08 KB

@ytsurk i've improved patch a bit, most importantly we are copying .browserslistrc from the first parent theme and not by default from bs_bootstrap. This makes more sense i think and allows more flexibility in custom child base themes. I also removed checks in update function because if the target does not exist this function will not be called in first place at all. Can you please check/test new version.

> I also started the documentation here

Thx, i created a new subsection https://www.drupal.org/docs/8/themes/bs-base/theme-development and moved this page to there.

ytsurk’s picture

ytsurk
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me :D

I tested updating and creating themes.

pivica’s picture

pivica commented
Assigned: ytsurk » Unassigned

Thx for checking.

  • pivica committed ee251c4 on 8.x-1.x authored by ytsurk 
    Issue #3066987 by ytsurk, pivica: Move Autoprefixer browsers gulp-option...
pivica’s picture

pivica commented
Status: Reviewed & tested by the community » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

sasanikolic’s picture

sasanikolic commented

Shouldn't we also edit gulp-options.yml in $child_themes and remove autoprefixer.browsers option from there?

It is possible that plugin will complain again if it gets this option?

This is exactly what happened in one of our projects. I think the update function should remove this configuration if available... Although, what if the configuration is different from the default one? Then we should somehow convert it to .browserslistrc configuration.

pivica’s picture

pivica commented

> Then we should somehow convert it to .browserslistrc configuration.

That is way too much work, we are still in alpha and this settings are rarely or never changed by child themes. If somebody wants to create update function for this feel free to create a follow-up issue and provide a patch.