Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure .browserlistrc reflects our browser support #42279

Closed
jfsiii opened this issue Jul 30, 2019 · 14 comments · Fixed by #71876
Closed

Ensure .browserlistrc reflects our browser support #42279

jfsiii opened this issue Jul 30, 2019 · 14 comments · Fixed by #71876
Assignees
Labels
discuss Team:Operations Team label for Operations Team

Comments

@jfsiii
Copy link
Contributor

jfsiii commented Jul 30, 2019

The current . browserslistrc uses "last 2 versions, > 5%, Safari 7" matches these browsers:

https://browserl.ist/?q=last+2+versions%2C+>+5%25%2C+Safari+7
Screen Shot 2019-07-30 at 1 28 00 PM

Is this the desired set of browsers we wish to support? Looking at our browser support matrix, it seems we don't support IE 10, but it's included in the set of browsers above.

These settings are used by CSS & JS transform tools and can have a large impact on the size of our bundles and time to create them. Some more info about the "last 2 versions" settings can be found here

I'd like to discuss what browsers we want to include (any beyond those shown in the matrix) and any we want to exclude, then create the appropriate browser list settings to achieve that set.

Some examples to get the discussion started:

A) What I think we intended

"last 2 Chrome versions, last 2 Safari versions, last 2 Firefox versions, last 2 Edge versions, ie 11, > 5%, Safari 7"
Screen Shot 2019-07-30 at 2 17 16 PM

5% is a pretty high market share

B) Same as above, minus any market share rule

"last 2 Chrome versions, last 2 Safari versions, last 2 Firefox versions, last 2 Edge versions, ie 11, Safari 7"

Screen Shot 2019-07-30 at 2 19 09 PM

C) Combine (A) with the recommended values from https://jamie.build/last-2-versions

"last 2 Chrome versions, last 2 Safari versions, last 2 Firefox versions, last 2 Edge versions, ie 11, Safari 7, > 0.25%, not op_mini all"

Screen Shot 2019-07-30 at 2 25 35 PM

There are many more options, I just used these to get us talking about what we do / don't want to include.

@jfsiii jfsiii added discuss Team:Operations Team label for Operations Team labels Jul 30, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@epixa
Copy link
Contributor

epixa commented Jul 30, 2019

Can you include the differences in bundle sizes (or any other measurements you think are relevant) between the options?

@tylersmalley
Copy link
Contributor

If you open up a PR, you can access the Linux build artifact to compare the bundle sizes if that is easier for you.

@streamich
Copy link
Contributor

I would like to see not dead added.

@streamich
Copy link
Contributor

Here is one example: > .5% or last 2 versions and not dead.

@spalger
Copy link
Contributor

spalger commented Jul 30, 2019

Best I can remember I just copied the browser list from our old autoprefixer config years ago. Thanks for taking a look at this!

If the artifact size difference between B and C isn't that much different I'd prefer C.

@jfsiii
Copy link
Contributor Author

jfsiii commented Jul 30, 2019

@spalger Yeah, that's how I read it. A nice conservative approach which didn't change the rules and consolidated them into one place.

Now that they're all sharing the same rules, I wanted to confirm that those rules are what we desired.

@epixa
Copy link
Contributor

epixa commented Jul 30, 2019

In practice, "last 2 versions" isn't super relevant here. If we're going to be tweaking this, I think our build process should explicitly support the ESR versions onward for Firefox. Based on browserl.ist, the two most popular Firefox versions are current-1 (67) and ESR (60). AFAIK, Chrome doesn't have any sort of ESR/LTS version, but if I'm mistaken we should do the same for that.

@jfsiii
Copy link
Contributor Author

jfsiii commented Jul 30, 2019

In practice, "last 2 versions" isn't super relevant here

Can you expand on that?

Based on browserl.ist, the two most popular Firefox versions are current-1 (67) and ESR (60). AFAIK, Chrome doesn't have any sort of ESR/LTS version, but if I'm mistaken we should do the same for that.

They have a Firefox ESR option. We're not currently targeting that, but we can add it. If we want, we can even base the values off our our analytics (e.g. > 1% in my stats, > 5% in US).

See the full list for other options.

I'll open a PR and we can iterate there

@jfsiii
Copy link
Contributor Author

jfsiii commented Jul 31, 2019

TL;DR: I think we can close this as "confirmed" and circle back to it at a point when we drop support for IE11 or use one of the patterns for serving different bundles to modern and older browsers

I opened a draft PR which was basically "(A) + Firefox ESR" but the bundle sizes were nearly identical when I tested in the browser.

I believe the main issue is that IE11 requires so many polyfills that as long as it's included in the same bundle with the modern/evergreen browsers, we won't see any real gains.

Here's a gist of the output with the babel debug flag enabled.

Of the 6337 lines (which include whitespace, status messages, etc), 4446 or 70% are only for IE11 { "ie":"11" }. 80%+ include "ie":"11". (e.g. proposal-unicode-property-regex { "edge":"17", "firefox":"60", "ie":"11" })

And that's even with 30+ line sections like this where it's only included once

Using targets:
{
  "chrome": "74",
  "edge": "17",
  "firefox": "60",
  "ie": "11",
  "ios": "12",
  "safari": "12"
}

Using modules transform: cjs

Using plugins:
  transform-template-literals { "ie":"11", "ios":"12", "safari":"12" }
{
  "chrome": "74",
  "edge": "17",
  "firefox": "60",
  "ie": "11",
  "ios": "12",
  "safari": "12"
}

Using modules transform: cjs

Using plugins:
{
  "chrome": "74",
  "edge": "17",
  "firefox": "60",
  "ie": "11",
  "ios": "12",
  "safari": "12"
}

As I TL;DR'd I think the real gains are in separating IE11 and other legacy browsers from the modern/evergreen browsers and that's outside the scope of this ticket.

Please comment/reference any tickets regarding new bundling approaches (I assume related to NP). I'd love to watch and contribute there.

@jfsiii jfsiii closed this as completed Jul 31, 2019
@streamich
Copy link
Contributor

streamich commented Aug 2, 2019

@jfsiii

I understand that IE11 needs a bunch of prefixing, but so does IE10. Our current configuration includes IE10, excluding that would reduce probably few thousand lines of CSS.

Can we at least add not dead to our configs? So it would exclude IE10 and Blackberry browsers, that add a ton of prefixing.

@streamich streamich reopened this Aug 2, 2019
@epixa
Copy link
Contributor

epixa commented Aug 2, 2019

@streamich Can you add some concrete numbers to your statements here, particularly in terms of bundle size? I'm sure IE10 adds some more to our bundles, but "excluding that would reduce probably few thousand lines of CSS" seems to directly contradict what @jfsiii discovered when they checked the actual bundle sizes after removing IE10 support.

@streamich
Copy link
Contributor

@epixa I don't see in this thread @jfsiii specifically considering removing IE10 and providing any numbers on that, "seems to directly contradict what @jfsiii discovered when they checked the actual bundle sizes after removing IE10 support" — where can I see those bundle size numbers?

Our current CSS build is 30K lines of CSS

Screenshot 2019-08-03 at 18 42 20

If you simply add not dead to the config, CSS shrinks down to 25K lines

Screenshot 2019-08-03 at 18 44 15

30K - 25K = 5K, hence your quote "excluding that would reduce probably few thousand lines of CSS".

@jfsiii
Copy link
Contributor Author

jfsiii commented Aug 5, 2019

@streamich I did remove IE 10 in my PR but that's not to say that my tests or conclusions are without issue.

Here are the before & after screenshots I took:

All master kibana-all-master PR kibana-all-pr
XHR masterkibana-xhr-master PR kibana-xhr-pr
JS masterkibana-js-master PR kibana-js-pr
CSS masterkibana-css-master PR kibana-css-pr

the bundle sizes were nearly identical

I wrote this instead of "the bundles sizes barely changed, if at all. CSS was the only one that moved and it was about ~1k."

Looking back at those images now, I see that the compressed CSS size changed about 100k. 1.5 MB vs 1.4 MB. This was a more drastic config change than only adding not dead but it's a big drop.

Sorry for not posting these earlier for others to see. To be totally transparent, I started this on a Thursday afternoon, was doing this Thursday evening, and had a feature I really wanted to concentrate on Friday. So when the results where underwhelming but seemingly correct (debug output had expected browsers, and I saw how we were loading all plugins on the homepage), documented my thoughts and closed this so as not to performance/webpack nerd snipe myself.

@mistic mistic self-assigned this Mar 30, 2020
@tylersmalley tylersmalley changed the title Confirm .browserlistrc settings Ensure .browserlistrc reflects our browser support Mar 30, 2020
@tylersmalley tylersmalley assigned tylersmalley and unassigned mistic Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Operations Team label for Operations Team
Projects
No open projects
7 participants