@tsteur opened this Pull Request on October 21st 2021 Member

Description:

Fix https://github.com/matomo-org/matomo/issues/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

@diosmosis commented on October 21st 2021 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 commented on October 21st 2021 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 commented on October 21st 2021 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: https://github.com/browserslist/browserslist/issues/316

@tsteur commented on October 21st 2021 Member

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 commented on October 21st 2021 Member

@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 commented on October 21st 2021 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 commented on October 21st 2021 Member

@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 commented on October 31st 2021 Member

@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 commented on October 31st 2021 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 commented on October 31st 2021 Member

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

@diosmosis commented on October 31st 2021 Member

@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 commented on October 31st 2021 Member

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

@diosmosis commented on November 1st 2021 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 commented on November 1st 2021 Member

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 https://github.com/browserslist/browserslist/issues/316 could be good additionally and we could update the browserlist in major releases or so?

@diosmosis commented on November 1st 2021 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 :+1:

@diosmosis commented on November 1st 2021 Member

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

@tsteur commented on November 2nd 2021 Member

Be great @diosmosis

@diosmosis commented on November 2nd 2021 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 commented on November 6th 2021 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 commented on November 6th 2021 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 commented on November 7th 2021 Member

Awesome. Can this one be reviewed @diosmosis ?

@diosmosis commented on November 8th 2021 Member

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

This Pull Request was closed on November 8th 2021
Powered by GitHub Issue Mirror