- Notifications
You must be signed in to change notification settings - Fork20.6k
Core: Migrate from AMD to ES modules 🎉#4541
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
ES modules bindings are immutable and we relied on the `ajax/var/nonce.js`variable being mutable. Workaround it by creating a wrapper object.We might try to compress it somehow in the future.
The export:```jsexport default jQuery.fn.delay;```was being cut out by Rollup as it was unused but due to the possibility ofthe `jQuery.fn.delay` access having side effects (e.g. via getters) it leftthe following line in:```jsjQuery.fn.delay;```This fails our ESLint setup and takes unnecessary space so this commit removesthe export.
Test files themselves are still loaded via RequireJS as that has to work inIE 11.
…work)Custom compilations (excluding/including some modules) is not supported yet.
When custom compilation is used, the version string can get large.
This is necessary so that excluding some non-core modules doesn't exclude"core/parseHTML.js".
mgol commentedNov 14, 2019
Uh oh!
There was an error while loading.Please reload this page.
This PR now adds 89 bytes to the minified (non-gzipped) build & 5 bytes to the minified gzipped build. The gzipped difference mostly comes from the addition of the I'm OK with such a delta given the gains from using the modern module format & removing our brittle compile process hacks as it's now mostly relying on Rollup but maybe it's possible to optimize it a little. |
OK, apparently we can go back to -10 bytes minified gzipped (& minimal unminified diff from the current Git compilation) if we move the |
gibson042 approved these changesNov 18, 2019
Member
gibson042 left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
This is so beautiful! 😍
Uh oh!
There was an error while loading.Please reload this page.
mgol added a commit to mgol/jquery that referenced this pull requestNov 20, 2019
jQuery source is now authored in ECMAScript modules. Native browser support forthem requires full file names including extensions. Rollup works even if importpaths don't specify extensions, though, so one import slipped through withoutsuch an extension, breaking native browser import of src/jquery.js.A new ESLint rule using eslint-plugin-import prevents us from regressing on thatfront.Refjquerygh-4541Ref0753201
2 tasks
mgol added a commit that referenced this pull requestNov 25, 2019
jQuery source is now authored in ECMAScript modules. Native browser supportfor them requires full file names including extensions. Rollup works evenif import paths don't specify extensions, though, so one import slippedthrough without such an extension, breaking native browser import ofsrc/jquery.js.A new ESLint rule using eslint-plugin-import prevents us from regressingon that front.Also, eslint-plugin-import's no-cycle rule is used to avoid import cycles.Closesgh-4544Refgh-4541Ref0753201
4 tasks
mgol added a commit to mgol/jquery that referenced this pull requestNov 27, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull requestNov 27, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull requestNov 27, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull requestNov 30, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull requestNov 30, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull requestNov 30, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.A "Load with AMD" checkbox was also restored to the QUnit setup. Note that,contrary to jQuery 3.x, AMD files need to be generated via `grunt amd` or`grunt` as sources are not authored in ECMAScript modules. To achieve a similarno-compile experience during jQuery 4.x testing, use the new "Load as modules"checkbox which works in all supported browsers except for IE & Edge (the legacy,EdgeHTML-based one).
mgol added a commit to mgol/jquery that referenced this pull requestNov 30, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.A "Load with AMD" checkbox was also restored to the QUnit setup. Note that,contrary to jQuery 3.x, AMD files need to be generated via `grunt amd` or`grunt` as sources are not authored in ECMAScript modules. To achieve a similarno-compile experience during jQuery 4.x testing, use the new "Load as modules"checkbox which works in all supported browsers except for IE & Edge (the legacy,EdgeHTML-based one).
mgol added a commit to mgol/jquery that referenced this pull requestNov 30, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.A "Load with AMD" checkbox was also restored to the QUnit setup. Note that,contrary to jQuery 3.x, AMD files need to be generated via `grunt amd` or`grunt` as sources are not authored in ECMAScript modules. To achieve a similarno-compile experience during jQuery 4.x testing, use the new "Load as modules"checkbox which works in all supported browsers except for IE & Edge (the legacy,EdgeHTML-based one).
mgol added a commit to mgol/jquery that referenced this pull requestNov 30, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.A "Load with AMD" checkbox was also restored to the QUnit setup. Note that,contrary to jQuery 3.x, AMD files need to be generated via `grunt amd` or`grunt` as sources are not authored in ECMAScript modules. To achieve a similarno-compile experience during jQuery 4.x testing, use the new "Load as modules"checkbox which works in all supported browsers except for IE & Edge (the legacy,EdgeHTML-based one).
mgol added a commit to mgol/jquery that referenced this pull requestDec 2, 2019
jQuery source has been migrated injquerygh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.A "Load with AMD" checkbox was also restored to the QUnit setup. Note that,contrary to jQuery 3.x, AMD files need to be generated via `grunt amd` or`grunt` as sources are not authored in ECMAScript modules. To achieve a similarno-compile experience during jQuery 4.x testing, use the new "Load as modules"checkbox which works in all supported browsers except for IE & Edge (the legacy,EdgeHTML-based one).
mgol added a commit that referenced this pull requestDec 9, 2019
jQuery source has been migrated ingh-4541 from AMD to ES modules. To maintainsupport for consumers of our AMD modules, this commits adds a task transpilingthe ES modules sources in `src/` to AMD in `amd/`.A "Load with AMD" checkbox was also restored to the QUnit setup. Note that,contrary to jQuery 3.x, AMD files need to be generated via `grunt amd` or`grunt` as sources are not authored in ECMAScript modules. To achieve a similarno-compile experience during jQuery 4.x testing, use the new "Load as modules"checkbox which works in all supported browsers except for IE & Edge (thelegacy, EdgeHTML-based one).Refgh-4541Closesgh-4554
mgol added a commit to mgol/jquery that referenced this pull requestFeb 24, 2020
This commit aligns the `3.x-stable` branch with `master` in two aspects:1. It migrates the nonce module to return an object instead of a primitivevariable. This had to be changed on `master` as in ES modules you exportlive read-only bindings to variables, meaning you can't increment the noncedirectly. Also, the way it was done so far was working differently in AMD & thesingle built file - in the built file one nonce variable was declared, accessedand incremented. In AMD mode separate instances were create for each modulethat depend on the nonce module, creating unintended nonce clashes.2. Whether the `noGlobal` parameter was set to `true` is now checked using thetypeof operator to align with `master`.Refjquerygh-4541Refd0ce00c
mgol added a commit that referenced this pull requestFeb 24, 2020
This commit aligns the `3.x-stable` branch with `master` in two aspects:1. It migrates the nonce module to return an object instead of a primitivevariable. This had to be changed on `master` as in ES modules you exportlive read-only bindings to variables, meaning you can't increment the noncedirectly. Also, the way it was done so far was working differently in AMD & thesingle built file - in the built file one nonce variable was declared, accessedand incremented. In AMD mode separate instances were create for each modulethat depend on the nonce module, creating unintended nonce clashes.2. Whether the `noGlobal` parameter was set to `true` is now checked using thetypeof operator to align with `master`.Closesgh-4612Refgh-4541Refd0ce00c
rart commentedApr 3, 2020
@mgol when will this effort be published to npm? |
@rart This is planned for jQuery 4.0.0 which will take at least several more months. Migrating this to the |
rart commentedApr 4, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@mgol thanks for the quick reply. Have you guys considered an |
@rart There is still significant work planned for jQuery 4.0.0 so we don't plan any pre-release soon; it'd be too far from the final release. If you want to use ES modules from |
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
Summary
Migrate source to ES modules. RequireJS is still used to load tests, this may perhaps be changed in a separate PR.
Reviewing by commit might be the easiest.
@timmywil I'd appreciate especially a review of the build process after my changes as custom compilation works slightly differently now (for example, I had to add an explicit import of
./core/parseHTML.jstojquery.jsas otherwise excluding AJAX cut out that module as well and a lot depends on it). If you could locally test some scenarios you had in mind when you initially wrote that & make sure they still work, that'd help.TODO:
amd/Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com