Browserslistc and Android

[XhmikosR]

As I discovered in #30986, if I remove Android from .browserslistrc tests fail on Legacy Edge. For example: https://github.com/twbs/bootstrap/runs/788301337?check_suite_focus=true

This might be an issue with Babel, opening an issue to track it.

/CC @hzoo

[XhmikosR]

@hzoo we dropped Legacy Edge support in our main branch, but not when I remove Android from browserslist then Safari 10.0 fails: https://github.com/twbs/bootstrap/runs/1399985635

Any hints how to proceed with this? I'd like to get any browserslist-related changes in before we release v5.0.0-beta1 :)

[mdo]

@XhmikosR Anything still to do here since we're not supporting Legacy Edge?

[XhmikosR]

Yeah, the issue is still present and we still have Android >= 6.

[aavmurphy]

The debug:true option in .babelrc shows which browser version causes the use of which babel plugin/polyfill.

[XhmikosR]

The problem is two-fold AFAICT:

1. should our dist files be ES5 or ES6? We are already using some new features already, but using less transpiled code will result in smaller dist files
2. there's still the failures on Safari which I don't get why they happen

[XhmikosR]

So, it seems the error we are hitting is Safari 10 throws an "Undefined is not a function" error in places like:

[].concat(...element.children)

[alpadev]

@XhmikosR hard to validate as I can't get Safari 10 to run on my macbook but [].concat(...document.body.children) throws indeed some error in Browserstack's Safari 10.1. Array.from(document.body.children) would work in that case.

Error:

[XhmikosR]

@alpadev thanks for testing it out :) That being said, I think the issue is that this should have been handled by Babel, shouldn't it?

Also, check out my comment above; I'm not sure if there's a convention about the dist files syntax. Traditionally, we were using ES5 or even ES3. Now, on main we do use some modern APIs already.

EDIT:

Here's a temp branch to see the size diff:

• https://github.com/twbs/bootstrap/compare/main-xmr-browserslist
• https://ja2r7.app.goo.gl/nJogapDpNshsgcUV6

The difference is quite big... Putting aside that it's probably a breaking change to land it now, though.

[alpadev]

😲 nice improvement. I wish reducing payload size was always that easy (too bad we can't get foresee the impact).

I agree with you that ideally Babel should take care of code being compliant with our targeted browsers.

Maybe related: babel/babel#11886 and babel/babel#8298 (Last comment here mentions this babel/rfcs#5)

About the output it's hard to decide I guess.. specially if one don't know about the requirements that users might have. Maybe we should consider creating two separate bundles? One that is es5 compatible but bigger and one that targets modern browsers and is smaller?

[XhmikosR]

TBH, there shouldn't be any real impact because our supported browsers support these features. Hence why my initial plan was to move ahead with this change, but I kept hitting the test failures.

Now, I have no idea if it's something in our Babel config, a bug upstream or something else. So, I kept this change back. And technically, I'm not sure if we can land the change in beta3, even if we sorted out the test failures.

[alpadev]

To my understanding using Babel in "loose-mode" (that's what we do I think) might result in non spec compliant code when it comes to using the spread operator with certain things.

"Well, this is a cost of using loose mode. It's not always spec-compliant and such cases might bite you."
"Loose mode is meant to work only in a specific subset of all the possible cases"

About those tests in Safari 10.. Safari 11 was released 31.10.2017.. Considering macs usually get around 7 years of OS updates that means one needs to use a mac that is like 10+ years old to be incompatible. According to https://caniuse.com/usage-table the percentage is quite low too.

Hard to say what would be the best thing to do =/ That's like the fun we frontend guys always had with the rapid development the last decade..

[alpadev]

We could replace [].concat(...element.children) with Array.from(element.children). The support is quite good https://caniuse.com/?search=array.from

Hard to propose anything because I'd usually make such decision based on user statistics and requirements for my project. IMO if you really want to be on the safe side we would create two separate bundles. bootstrap.bundle.js and bootstrap.bundle.compat.js or something. But thats just my 2 cents..

Maybe you want to create some internal poll how to proceed with that so it's not your sole responsibility 🙂

[alpadev]

@XhmikosR So I investigated this topic a little further. Also I have to say thank you for giving me a reason to do this.

Babel always been kinda blackbox for me TBH. You set it up. Maybe got to fiddle with the configuration because of outdated informations. Eventually you're happy with the result but after a while someone comes up with a problem that is related to this (usually some old IE in my case).

So that we're on the same boat - Babel should transform our code so it's fully compliant with our targeted browsers. Although with our current configuration Babel isn't fully doing this.

One reason for this is that we're using "loose mode" (source). In loose mode, the transformations do not fully conform to the ecma spec and usually result in a much simpler implementation that may not behave as intended or respect all cases. The advantage is that the resulting code is less complex and significantly smaller (~15kb for our bundle).

To pick up your example from above.
Source:

children(element, selector) {
  return [].concat(...element.children)
    .filter(child => child.matches(selector))
}

loose: true

children: function children(element, selector) {
  var _ref2;

  return (_ref2 = []).concat.apply(_ref2, element.children).filter(function (child) {
    return child.matches(selector);
  });
}

loose: false

children: function children(element, selector) {
  var _ref2;

  return (_ref2 = []).concat.apply(_ref2, _toConsumableArray(element.children)).filter(function (child) {
    return child.matches(selector);
  });
}

function _toConsumableArray(arr) {
  return _arrayWithoutHoles(arr) || _iterableToArray(arr) || _unsupportedIterableToArray(arr) || _nonIterableSpread();
}

// ...

Another reason. For browsers that don't support some specific JS feature, Babel can be configured to automatically add polyfills but in our case this isn't happening as well. (https://babeljs.io/docs/en/babel-preset-env#usebuiltins)

About the Browserslist configuration. Babel is resolving targeted browsers with the option mobileToDesktop: true (source).

From the Browserslist docs:

mobileToDesktop: Use desktop browsers if Can I Use doesn’t have data about this mobile version. For instance, Browserslist will return chrome 20 on and_chr 20 query (Can I Use has only data only about latest versions of mobile browsers). Default is false.

Some background to the Android Version according to Can I Use

Android browser/WebView version numbers through 4.4 refer to the version of Android OS. Support listed is for the Android core; it should be noted that many hardware vendors (Samsung, HTC, etc.) use altered version of their default browser which may include more/less/buggy support. Starting in Android 5, the web engine can be updated separately, so the latest Chromium version number is used instead.

In our case with Android >= 6 Browserslist returns every Android WebView/Chromium version starting from 37 (actually 36 because there is a small bug that is caused by Google skipping v82 for Chrome) since thats what they use as the lowest version (source). And it looks like that Babel handles those versions equally in some cases (source). I could be wrong but it looks like we're effectively targeting Chrome starting from 37 not 60.

You can run npx browserslist --mobile-to-desktop to get a list of all browsers that are targeted.

[aavmurphy]

If you set the debug option in .babelrc, it shows which babel plugin is being activated by which browser versions, e.g.
proposal-numeric-separator { "ios":"10.3" }

If a plugin is being activated by just 1 line of code, it can be easier to just rewrite it.

Unless you have useBuiltins set (which .babelrc does not), babel only does transpiling (e.g. let => var) and not polyfills. Setting useBuiltins and debug shows which polyfill plugins are activated.

In practise, I've found it easier to avoid polyfills, e.g. in for loops, not using the array.entries() iterator.

Aside: in .browserlistrc, setting > 0.5% means the babeljs plugins activated will change over time. See npx browserslist@latest --update-db

[XhmikosR]

@alpadev so this means it's expected that we get this result since we are using loose: true, right?

I could be wrong but it looks like we're effectively targeting Chrome starting from 37 not 60.

That is correct, it's a side effect of using Android >= 6 since Chrome is used there too.

Not sure how to proceed at this point. I mean, I definitely don't want us to disable loose, nor use polyfills. If we could work around these, that would be the best. That being said, I'm not sure if using modern stuff in our dist code will cause any issues for consumers. Technically, we don't support browsers which don't support these features, but you never know...

My initial plan was to make our dist files as modern as possible for v5, but since I had trouble for months, so, I slated the change for later.

[XhmikosR]

Alright, after discussing this with @mdo, we agreed to proceed with dropping Safari < 12, thus we should be able to get dist files with modern code. I'll prepare a PR tomorrow.

[alpadev]

@aavmurphy you're correct. Adding debug: true definitely helps with finding those spots but that's rather some manual process and ideally it should be some set and forget thing IMO. I'm also not a huge fan of polyfills but I guess it depends on what is required/expected and in some cases they may be mandatory.

@XhmikosR Yeah loose: true could have been the culprit in case of Safari 10. In general even for some simple code the loose transformations can have a different/unwanted output because the implementation may be too simple and/or not getting the intention right. (e.g. here). Not saying we should disable loose mode as there is usally just a little change required to make it work and the fully spec compliant transformations are likely to bloat our code when it isn't necessary. (Some article in case you're still unsure what loose does.)

To further optimize Browserslist to our needs, we could utilize the supports query feature

supports es6-module: browsers with support for specific features. es6-module here is the feat parameter at the URL of the Can I Use page. A list of all available features can be found at caniuse-lite/data/features. (source)

Also we could utilize https://github.com/browserslist/browserslist#configuring-for-different-environments and create a modern and a compatiblity bundle just in case you have concerns about the support.

In the end Babel uses the informations provided by Browserslist to decide what transformations are necessary so the better it matches our requirements, the lesser the chances are, that there are changes to our code that could introduce errors.

Hope that helps.

[XhmikosR]

I tried loose: false, and it still fails on Safari 10...

I think we can afford just targeting Safari > 12, see #33399. I mean, that was the initial idea for v5. What's I'm not sure about is if everything else is OK/following the best practices. Like our dist files, rollup config etc.

I mean, from the looks of it, we could even drop babel. I'd say that we try to make one change at a time. We can keep babel of course, since this will allow us to use features that aren't shipped to browsers yet, just pointing it out what I notice so far.

[aavmurphy]

It might be that the ( ) => { } syntax ("arrow functions") being replaced causing the problem.

• that's not ios & safari <10, not opera mini, and maybe not (some of the chinese ones)

npx browserslist@latest --update-db updates
npx browserslist
shows which browsers are being supported.

[XhmikosR]

We don't support those browsers and the issue we are having is specific to spread and Safari.

…

On Sat, Mar 20, 2021, 15:59 Andrew Murphy ***@***.***> wrote: It might be that the ( ) => { } syntax ("arrow functions") being replaced causing the problem. - that's not ios & safari <10, not opera mini, and maybe not (some of the chinese ones) npx ***@***.*** --update-db updates npx browserslist shows which browsers are being supported. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31153 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACVLNPS3NGN3TG66MKVPSTTESS3JANCNFSM4OFUDHNQ> .

[alpadev]

I tried loose: false, and it still fails on Safari 10...

Yeah sorry, from looking at the changes in the code I've been guessing that loose: true could have been the problem but I wasn't really able to validate that. Also I just realized the differences in the code I mentioned here were actually caused by Android >= 6.

I just managed to install MacOS Yosemite on a VM and was able to install Safari 10.1.2 there. I can confirm that with loose: false Safari is still throwing the error from the screenshot I posted here. But after enabling Babel to add polyfills as well, I got Bootstrap to run in Safari 10.1.2 without errors (both, with loose: true and loose: false).

I think we can afford just targeting Safari > 12, see #33399. I mean, that was the initial idea for v5.

You're likely right about this. Safari 12 is able to be installed on MacOS Sierra (released in September 2016) up to Mojave (September 2018). Looks like Apple isn't really into supporting older versions of their browser but rather to keep them in line with their OS major releases once a year.

What's I'm not sure about is if everything else is OK/following the best practices. Like our dist files, rollup config etc.

TBH I'm not very experienced with rollup because I've been using webpack most of the time. When I fiddled around to enable polyfills in Babel I ran into a problem tho, where I had to enable the commonjs plugin for the bundle too. While reading the documentations I realized that in their examples they have the node-resolve plugin always going first but in our configuration it comes after the babel plugin. Since everything been working so far I'm not sure if it really matters tho.

I mean, from the looks of it, we could even drop babel. I'd say that we try to make one change at a time. We can keep babel of course, since this will allow us to use features that aren't shipped to browsers yet, just pointing it out what I notice so far.

I really have no preferences about Babel. I usually add it to my build stack because it's mentioned in a lot of articles and for the lack of native support for ES6 in the past but I also had my moments when it wouldn't do what I expected. One thing to mention about features that are partly shipped yet. There is another configuration flag https://babeljs.io/docs/en/babel-preset-env#shippedproposals. In general I'm with you - if we consider dropping it we should do this step by step and the debug option can help with narrowing down what browser support we're giving up. If you find it useful I can also post a list with all Babel transformations and their browser versions.

[aavmurphy]

Huh, so it was a missing polyfill after all.

It's important to enable polyfills with debug set to see what else Babel is trying to "fix" for which browser. (If useBuiltIns isn't set, Babel silently ignores polyfills)

If it is just the spread ( ...array ) operator in one location, it might be simpler replacing it with old javascript - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax

If so, not iOS < 10 would be a good compromise. Having ios >= 10.3 allows let, classes and () => {} functions (which preserve this). This gives 95% coverage (according to caniuse)