-
Notifications
You must be signed in to change notification settings - Fork 333
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
Add Babel to transform ES2020 public and static class fields #3820
Conversation
84246cc
to
5e9495d
Compare
31d01ac
to
ab05f22
Compare
ab05f22
to
b53b6bb
Compare
5e9495d
to
caa3839
Compare
b53b6bb
to
7bba1fd
Compare
00ead49
to
d26002e
Compare
7bba1fd
to
0c1c100
Compare
d26002e
to
775110d
Compare
0c1c100
to
1f83c7c
Compare
646916f
to
1bd3aba
Compare
1f83c7c
to
6eb8772
Compare
1bd3aba
to
7c80453
Compare
6eb8772
to
8ea3e97
Compare
7c80453
to
20452d5
Compare
8ea3e97
to
869dd36
Compare
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.
Diff looks good to me - time to use all the shiny things!
As noted above, we're holding off merging this till Exit This Page gets merged.
Thanks @domoscargin My only concern (now) is it's slightly jarring seeing JSDoc comments moved to the bottom of the class @36degrees @romaricpascal Any further thoughts on removing comments to keep KB size down? Hadn't realised how easy it was with Babel Comments can use Seeing ~40-50% drop in file size KB |
b191eb3
to
932151a
Compare
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.
Neat ! And thanks for the review @domoscargin 🙌🏻 I'm pretty keen to remove unnecessary comments (especially given the gains highlighted by the spike), but let's discuss that as a separate PR so we can have this merged on its own 😊
|
||
/** @private */ | ||
boundOnHashChange | ||
|
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.
question Noticed while looking at #3958 that this list is missing the mql
field, which is still first used in setupResponsiveChecks
a little further. Is it intentional, or just that is was missed among all the other fields? 😊
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.
@romaricpascal Nice spot, yeah it was missed. Added it 👍
Transforms will be applied based on `.browserslistrc` supported browsers, with helper functions bundled if needed
We enable `{ bugfixes: true }` to let Babel use smaller transforms for partially supported browser features For example, Babel knows that full support for template strings as “tagged templates with escape sequences” only shipped in Safari 11 But we can now use template strings in Safari 9.1 too https://caniuse.com/mdn-javascript_grammar_template_literals_template_literal_revision
Babel transpiles ES2020 class fields for our Browserslist supported browsers
5fdd10b
to
ae0bfb8
Compare
Shows the compiler that we’re using DOM timers and intervals, rather than Node.js globals
The compiler can infer construction types, reducing JSDoc comments
ae0bfb8
to
bb9e80f
Compare
@romaricpascal @domoscargin Updated with Exit this page from #3953 |
Transformation looks fine to me on the file for Exit this page as well 🥳 |
This PR adds Babel transpilation as decided in:
I've added transforms for two ES2020 features back to ES2015—without polyfills or helper functions
Public static fields
After merging Migrate JavaScript to ES2015 classes we were left with "optional" class properties appended in the constructor that would be clearly defined as
static
in ES2020Before
After
Public fields
We also have "optional" class properties appended in the constructor that would be public class fields
Before
After
Both
$module
and$inputs
can be declared as class fields (with default values if required)