-
Notifications
You must be signed in to change notification settings - Fork 326
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
Update Browserslist config with future support #3728
Conversation
0d6ea90
to
5fb95e9
Compare
5fb95e9
to
3f75384
Compare
3f75384
to
e4946f6
Compare
e4946f6
to
84246cc
Compare
Out of curiosity, why does the JavaScript configuration specify:
and not just
As far as I can tell from playing with browsersl.ist, there isn't any difference between the browsers returned by the first query vs. the second query. |
Thanks for taking a look @querkmachine It was a safety net for the You'll see it unintentionally bringing in Safari on iOS 8.1-8.4, 9.3 etc where only Safari 9+ actually supports ES2015 modules, but worse still only Safari 11+ supports So by narrowing it down to |
84246cc
to
5e9495d
Compare
5e9495d
to
caa3839
Compare
caa3839
to
1ff6234
Compare
1ff6234
to
42e6a5a
Compare
42e6a5a
to
6e232af
Compare
6e232af
to
892ba24
Compare
0ab4cf3
to
bfafd2f
Compare
bfafd2f
to
62c1f76
Compare
cbbfee1
to
63a6daa
Compare
Revised to lower Safari 12 to Safari 11 to match our "cut the mustard" check: 'noModule' in HTMLScriptElement.prototype |
> 0.1% in GB and supports es6-module | ||
last 6 Chrome versions | ||
last 6 Firefox versions | ||
last 6 Edge versions | ||
last 2 Samsung versions | ||
Safari >= 9 | ||
iOS >= 9 | ||
Firefox ESR | ||
Safari >= 11 and not Safari < 11 | ||
iOS >= 11 and not iOS < 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the decisions we made on Tuesday, I'd expect this to just be:
[javascripts]
supports es6-module
By restricting it to only browsers with >0.1% marketshare
we're back in a place where we may ship JS that is not parseable in all browsers that support type="module"
?
Additionally, is there benefit in listing out specific browsers when we know they'll be covered by supports es6-module
anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@36degrees Of course! I'm torn on wanting to let browsers go from Grade A, to Grade B, to Grade C
Using the query > 0.1% in GB and supports es6-module
lets us gracefully (in future) go from 100% polyfill coverage, to letting older browsers slowly fail feature detection and drop down a grade
Let me know if you'd rather have a fixed point in time instead—I'll fit in with you
(Adding specific browsers was to prevent the sliding scale from going too far)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch up @36degrees
For safety let's go with supports es6-module
Should we find that Babel is adding some weighty transforms/plugins that we could instead guard with "feature detection", we'll exclude them in the Babel config instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most interesting side effect of this change is over in:
We're now seeing all let
and const
transformed back to var
for Safari < 11
😮
Odd, but let's dig further
Babel reports that the transform was applied by @babel/plugin-transform-block-scoping
to fix slowdowns in Safari. Appears that we set safari10: true
in Terser for the same reason
You might find ESBuild's explanation is easier to follow than the WebKit bug
Top-level
var
In JavaScript,
let
,const
, andclass
declarations all introduce TDZ checks while var declarations do not. Since bundling typically merges many modules into a single very large top-level scope, the performance impact of these TDZ checks can be pretty severe. Converting top-levellet
,const
, andclass
declarations intovar
helps automatically make your code faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When adding debug: true
to the Babel @babel/preset-env
config you'll see the following:
Using plugins:
transform-unicode-sets-regex { chrome < 112, edge < 112, firefox, ios, opera < 98, safari, samsung }
proposal-class-static-block { chrome < 94, edge < 94, firefox < 93, ios, opera < 80, safari, samsung < 17 }
proposal-private-property-in-object { chrome < 91, edge < 91, firefox < 90, ios < 15, opera < 77, safari < 15, samsung < 16 }
proposal-class-properties { chrome < 74, edge < 79, firefox < 90, ios < 15, opera < 62, safari < 14.1, samsung < 11 }
proposal-private-methods { chrome < 84, edge < 84, firefox < 90, ios < 15, opera < 70, safari < 15, samsung < 14 }
proposal-numeric-separator { chrome < 75, edge < 79, firefox < 70, ios < 13, opera < 62, safari < 13, samsung < 11 }
proposal-logical-assignment-operators { chrome < 85, edge < 85, firefox < 79, ios < 14, opera < 71, safari < 14, samsung < 14 }
proposal-nullish-coalescing-operator { chrome < 80, edge < 80, firefox < 72, ios < 13.4, opera < 67, safari < 13.1, samsung < 13 }
proposal-optional-chaining { chrome < 91, edge < 91, firefox < 74, ios < 13.4, opera < 77, safari < 13.1, samsung < 16 }
proposal-json-strings { chrome < 66, edge < 79, firefox < 62, ios < 12, opera < 53, safari < 12, samsung < 9 }
proposal-optional-catch-binding { chrome < 66, edge < 79, ios < 11.3, opera < 53, safari < 11.1, samsung < 9 }
transform-parameters { edge < 18, ios, safari }
proposal-async-generator-functions { chrome < 63, edge < 79, ios < 12, opera < 50, safari < 12 }
proposal-object-rest-spread { edge < 79, ios < 11.3, safari < 11.1 }
transform-dotall-regex { chrome < 62, edge < 79, firefox < 78, ios < 11.3, opera < 49, safari < 11.1 }
proposal-unicode-property-regex { chrome < 64, edge < 79, firefox < 78, ios < 11.3, opera < 51, safari < 11.1, samsung < 9 }
transform-named-capturing-groups-regex { chrome < 64, edge < 79, firefox < 78, ios < 11.3, opera < 51, safari < 11.1, samsung < 9 }
transform-async-to-generator { ios < 11, safari < 11 }
transform-template-literals { ios < 13, safari < 13 }
transform-function-name { edge < 79 }
transform-unicode-regex { ios < 12, safari < 12 }
transform-block-scoping { ios < 11, safari < 11 }
proposal-export-namespace-from { chrome < 72, edge < 79, firefox < 80, ios, opera < 60, safari, samsung < 11.0 }
syntax-dynamic-import
syntax-top-level-await
syntax-import-meta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! Any thoughts on what we should do?
I guess we might want to do some performance testing on the affected Safari versions to see what the impact would be of disabling that transformation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep the config for now and assume (future) Babel will save our bacon many more times 😓
But ideally let's see if Rollup avoids the issue already, since those Babel transforms might be 100% unnecessary. We might not even have any code that triggers the slow TDZ ("temporal dead zone") bug
This PR is good to go though
const browserslistEnv = !api.env('test') | ||
? 'javascripts' | ||
: 'node' | ||
|
||
return { | ||
presets: [ | ||
['@babel/preset-env', { | ||
browserslistEnv, | ||
loose: browserslistEnv !== 'node', | ||
|
||
// Transform ES modules for Node.js | ||
modules: browserslistEnv === 'node' ? 'auto' : false | ||
}] | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that all of the config for '@babel/preset-env' depends on the browsersListEnv
, I'm wondering if it might be simpler to just return two different objects? Especially as there's a lot of negation going on at the minute which can make it harder to follow.
const browserslistEnv = !api.env('test') | |
? 'javascripts' | |
: 'node' | |
return { | |
presets: [ | |
['@babel/preset-env', { | |
browserslistEnv, | |
loose: browserslistEnv !== 'node', | |
// Transform ES modules for Node.js | |
modules: browserslistEnv === 'node' ? 'auto' : false | |
}] | |
] | |
} | |
let presetEnvConfig | |
if (api.env('test')) { | |
presetEnvConfig = { | |
browsersListEnv: 'node', | |
modules: false | |
} | |
} else { | |
presetEnvConfig = { | |
browsersListEnv: 'javascripts', | |
loose: true | |
} | |
} | |
return { | |
presets: [ | |
['@babel/preset-env', presetEnvConfig] | |
] | |
} |
Or even:
const browserslistEnv = !api.env('test') | |
? 'javascripts' | |
: 'node' | |
return { | |
presets: [ | |
['@babel/preset-env', { | |
browserslistEnv, | |
loose: browserslistEnv !== 'node', | |
// Transform ES modules for Node.js | |
modules: browserslistEnv === 'node' ? 'auto' : false | |
}] | |
] | |
} | |
if (api.env('test')) { | |
return { | |
presets: [ | |
['@babel/preset-env', { | |
browsersListEnv: 'node', | |
modules: false | |
}] | |
] | |
} | |
} else { | |
return { | |
presets: [ | |
['@babel/preset-env', { | |
browsersListEnv: 'javascripts', | |
loose: true | |
}] | |
] | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you rather we did?
We could end up adding a third development
config with more lightweight transforms, since most development only happens in latest Chrome/Safari/Firefox
I'll have a think on Monday
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@36degrees I've removed !== 'node'
to help keep examples/webpack/babel.config.js
short and sweet
But only because we're going to need to duplicate the config for the govuk-frontend
package over in:
63a6daa
to
803a28d
Compare
This will help in future to manage supported browsers/environments from a single config per workspace
But we see `npx browserslist` has removed these browsers: * UC Browser for Android (India, Indonesia, China) * QQ Browser (China)
Samsung Internet `last 2 Samsung versions` already targets 6 months of updates
We use an ES2015 “cut the mustard” check `'noModule' in HTMLScriptElement.prototype` which was available in Safari 11+ https://caniuse.com/mdn-html_elements_script_nomodule This differs to the Service Manual (June 2022) which suggests testing in Safari for macOS 12+ and Safari for iOS 12.1+ https://www.gov.uk/service-manual/technology/designing-for-different-browsers-and-devices
Browsers also get `{ loose: true }` to use smaller transforms See: https://babeljs.io/docs/babel-preset-env#loose
This change configures a new `stylesheets` Browserslist environment for PostCSS (including Autoprefixer) which means we can selectively drop IE11 elsewhere in future
The default browsers that load our CSS differ to those that run our JavaScript, so we don’t need Babel transforms for IE11 and other legacy browsers We’ll start by targetting `supports es6-module` and can optionally exclude Babel transforms or plugins as required, should we prefer feature detection instead
803a28d
to
2ef3da3
Compare
We're now seeing all `let` and `const` transformed back to `var` for slowdowns affecting Safari < 11 See comment: #3728 (comment)
This change configures a new `stylesheets` Browserslist environment for PostCSS (including Autoprefixer) which means we can selectively drop IE11 elsewhere in future See: alphagov/govuk-frontend#3728
Support for IE11
-ms-*
vendor prefixes came up recently on Slack following 478af39Based on thinkings from @querkmachine in The future of browser support in GOV.UK Frontend (internal) I've put a draft PR together of some Browserslist config changes we might want to make
Up for discussion as part of:
Browserslist config environments
I've included separate Browserslist environments for JavaScripts, Stylesheets and Node.js:
For example:
[stylesheets]
restores Internet Explorer 11 to maintain-ms-*
vendor prefixes[javascripts]
removes Safari 9-10 that fails'noModule' in HTMLScriptElement.prototype
Babel config
browserslistEnv
For the webpack example, Babel also used the Browserslist config so I've set
browserslistEnv
to ensure it switches to the newjavascripts
environment. See the Babel spike for how we'd used the same approach forgovuk-frontend
too: