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

Mark IE11 as no longer supported browsers #18199

Merged
merged 10 commits into from Nov 8, 2021

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Oct 21, 2021

Description:

Fix #18011

Kept the other translation so we can reuse it in the future (which was the idea when we added it).

The notification will be no longer needed as they now will be seeing an unsupported browser error page.

@diosmosis I remember you mentioned there is a command to find out which browsers vue supports. I'd also adjust the other browser version numbers based on that but couldn't find anymore what the command was to get that information. Can you help me out there?

Review

@tsteur tsteur added this to the 4.6.0 milestone Oct 21, 2021
@diosmosis
Copy link
Member

@tsteur the command is documented here: https://developer.matomo.org/guides/asset-pipeline#browser-support

There's no command specifically for Vue, we'd probably have to use their code at a specific tag. The output of the above command is based on https://github.com/matomo-org/matomo/blob/4.x-dev/.browserslistrc .

@justinvelluppillai
Copy link
Contributor

@diosmosis @tsteur we might want to make that list more explicit rather than moving (ie remove rules like <1%, last 2 versions maybe)

@diosmosis
Copy link
Member

@justinvelluppillai see the linked documentation, you can run npx browserslist to see explicit versions. If we want to lock the browser versions used for a certain amount of time, we could follow an approach like: browserslist/browserslist#316

@tsteur
Copy link
Member Author

tsteur commented Oct 21, 2021

image

I reckon if it's possible it be great to lock in some browser versions or support them back a bit more as otherwise we might actually deprecate browsers pretty quickly? If easily possible could be good to support some even older versions? are 2 last versions an or or and when they are per line? I reckon or be fine like > 1% or last 2 versions vs maybe and be maybe too restrictive?

@diosmosis
Copy link
Member

diosmosis commented Oct 21, 2021

@tsteur I'm not sure if each line is anded together. If it was not, IE would still be included, BUT > 1% or last 2 versions provides different output than > 1% and last 2 versions. I think per line is more set logic, so unions (edited) unless it sees a not, then it excludes from the result set. Or can be specified though on a specific line I think, eg, > 1% or last 2 versions (though again, there's no difference from what's there now). We can also go w/ a lower percent than 1% (eg, > 0.2%), though it's not recommended since it helps older browsers stay popular.

Specific versions can be locked using the technique in the github issue I linked to above.

@justinvelluppillai
Copy link
Contributor

@diosmosis that's kind of a great use of package-lock.json! Still wonder though if being more explicit in our .browserslistrc is a better approach for us

@diosmosis
Copy link
Member

diosmosis commented Oct 21, 2021

@justinvelluppillai that means you'd have to periodically and manually determine which browsers match the (now unwritten) requirements which defeats the purpose of using browserslist.

@tsteur tsteur added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Oct 22, 2021
@tsteur
Copy link
Member Author

tsteur commented Oct 31, 2021

@diosmosis @justinvelluppillai would there be any harm to change the first line to > 0.05% instead of > 1%?

If I understand correctly, this way we would support a lot more versions:

and_chr 94
and_ff 92
and_qq 10.4
and_uc 12.12
android 94
android 4.4.3-4.4.4
baidu 7.12
chrome 95
chrome 94
chrome 93
chrome 92
chrome 91
chrome 90
chrome 89
chrome 88
chrome 87
chrome 86
chrome 85
chrome 84
chrome 83
chrome 81
chrome 80
chrome 79
chrome 78
chrome 76
chrome 75
chrome 74
chrome 70
chrome 69
chrome 61
chrome 56
chrome 49
edge 95
edge 94
edge 93
edge 92
edge 18
firefox 93
firefox 92
firefox 91
firefox 78
firefox 52
ios_saf 15
ios_saf 14.5-14.8
ios_saf 14.0-14.4
ios_saf 13.4-13.7
ios_saf 13.3
ios_saf 12.2-12.5
ios_saf 12.0-12.1
ios_saf 11.3-11.4
ios_saf 11.0-11.2
ios_saf 10.3
ios_saf 9.3
ios_saf 6.0-6.1
kaios 2.5
op_mini all
op_mob 64
opera 80
opera 79
opera 78
opera 73
safari 15
safari 14.1
safari 14
safari 13.1
safari 13
safari 12.1
samsung 15.0
samsung 14.0
samsung 13.0
samsung 12.0
samsung 11.1-11.2
samsung 7.2-7.4

in comparison to currently
image

I would expect that quite a few users be on some of these older versions once you aggregate them all. And if it's just a matter of changing the percentage it be easy to keep compatibility?

@justinvelluppillai
Copy link
Contributor

@tsteur the harm is bigger final build size from supporting older browsers with more polyfills and less modern features. I still favour a more explicit approach, just specifying which browsers we use because then it's very clear (but the package-lock suggestion @diosmosis made would work too)

@tsteur
Copy link
Member Author

tsteur commented Oct 31, 2021

@justinvelluppillai it might be good to check how much of a difference in file size it actually makes to support these?

@diosmosis
Copy link
Member

diosmosis commented Oct 31, 2021

@tsteur you can get an idea on 4.x-dev by:

  1. checking out a clean 4.x-dev, then running development:compute-js-asset-size
  2. changing browserslist, running ./console vue:build, then running development:compute-js-asset-size

The result will have to be extrapolated to get an idea of how much it will affect the finished migration.

EDIT: Note that it may not actually make much of a difference overall, since it's probably hard to find modern browsers that don't support the features we use.

@diosmosis
Copy link
Member

I can take a quick look later today if no one takes a look by then.

@diosmosis
Copy link
Member

@tsteur

4.x-dev no changes:

image

4.x-dev w/ 0.05%:

image

And just as a sanity check, 4.x-dev w/ 0.05% + including IE:

image

Also note, that it is trivial to revert a change to .browserlistrc, as long as no public commitments have been made.

@tsteur
Copy link
Member Author

tsteur commented Nov 1, 2021

Nice, that's effectively no change in size at all so we could easily define w/ 0.05%:?

Using then the package lock mentioned in browserslist/browserslist#316 could be good additionally and we could update the browserlist in major releases or so?

@diosmosis
Copy link
Member

@tsteur

Nice, that's effectively no change in size at all so we could easily define w/ 0.05%:?

Yes looks like it. Double checked on caniuse, and most of the old browsers listed still support all the newer functionality we need. Most of the unsupported things are CSS related or large features like shadow dom support.

Using then the package lock mentioned in browserslist/browserslist#316 could be good additionally and we could update the browserlist in major releases or so?

That should work 👍

@diosmosis
Copy link
Member

@tsteur would you like me to update the PR w/ browserslist changes?

@tsteur
Copy link
Member Author

tsteur commented Nov 2, 2021

Be great @diosmosis

@diosmosis
Copy link
Member

@tsteur actually I think the built JS size may end up greater since we don't auto-include polyfills (so we can have one JS files w/ polyfills instead of adding it to every plugin's UMD). I'll do cross browser testing and see what the result is.

@diosmosis
Copy link
Member

@tsteur updated so it supports most browsers at > 0.05% usage. I tested manually on the following browser versions:

  • edge 18
  • chrome 49
  • android 4.4
  • firefox 52
  • ios_saf 9.3 (not supported by vue, only 10+)
  • safari 11.1

I had to add one extra polyfill for Object.entries().

Asset sizes:

image

image

@diosmosis
Copy link
Member

browserslist is a dependency of @vue/cli-service so it will update when that package is updated, which can be done once per major version.

@tsteur
Copy link
Member Author

tsteur commented Nov 7, 2021

Awesome. Can this one be reviewed @diosmosis ?

@diosmosis diosmosis added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Nov 8, 2021
@diosmosis
Copy link
Member

@tsteur tests should be passing, put this into review.

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine for me. @diosmosis feel free to merge when it fits best for the other vue PRs

@diosmosis diosmosis merged commit 312d3a0 into 4.x-dev Nov 8, 2021
@diosmosis diosmosis deleted the revert-18037-ie-support-warning branch November 8, 2021 21:23
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update list of no longer supported browsers (IE11)
4 participants