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

Core: Migrate from AMD to ES modules 🎉 #4541

Merged
merged 20 commits into from
Nov 18, 2019
Merged

Core: Migrate from AMD to ES modules 🎉 #4541

merged 20 commits into from
Nov 18, 2019

Conversation

mgol
Copy link
Member

@mgol mgol commented Nov 14, 2019

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.js to jquery.js as 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:

  • Add AMD compilation to amd/
  • (optional) Add back AMD mode to QUnit

Checklist

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:
```js
export default jQuery.fn.delay;
```
was being cut out by Rollup as it was unused but due to the possibility of
the `jQuery.fn.delay` access having side effects (e.g. via getters) it left
the following line in:
```js
jQuery.fn.delay;
```
This fails our ESLint setup and takes unnecessary space so this commit removes
the export.
Test files themselves are still loaded via RequireJS as that has to work in
IE 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".
test/jquery.js Outdated Show resolved Hide resolved
@mgol
Copy link
Member Author

mgol commented Nov 17, 2019

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 ./core/parseHTML.js import to jquery.js, without it it added 36 bytes to minified (non-gzipped) and actually reduced (!) the minified gzipped build by 10 bytes. This is only a result of re-shuffling the final file, apparently Uglify was compiling the previous one better.

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.

@mgol
Copy link
Member Author

mgol commented Nov 17, 2019

OK, apparently we can go back to -10 bytes minified gzipped (& minimal unminified diff from the current Git compilation) if we move the ./core/parseHTML.js from directly after the ./core.js import to just before the ./ajax/load.js one.

Copy link
Member

@gibson042 gibson042 left a 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! 😍

src/ajax.js Outdated Show resolved Hide resolved
@mgol mgol removed the Needs review label Nov 18, 2019
@mgol mgol changed the title Core: Migrate to ES modules Nov 18, 2019
@mgol mgol merged commit d0ce00c into jquery:master Nov 18, 2019
@mgol mgol deleted the es-modules branch November 18, 2019 20:15
mgol added a commit to mgol/jquery that referenced this pull request Nov 20, 2019
jQuery source is now authored in ECMAScript modules. Native browser support for
them requires full file names including extensions. Rollup works even if import
paths don't specify extensions, though, so one import slipped through without
such an extension, breaking native browser import of src/jquery.js.

A new ESLint rule using eslint-plugin-import prevents us from regressing on that
front.

Ref jquerygh-4541
Ref 0753201
mgol added a commit that referenced this pull request Nov 25, 2019
jQuery source is now authored in ECMAScript modules. Native browser support
for them requires full file names including extensions. Rollup works even
if import paths don't specify extensions, though, so one import slipped
through without such an extension, breaking native browser import of
src/jquery.js.

A new ESLint rule using eslint-plugin-import prevents us from regressing
on that front.

Also, eslint-plugin-import's no-cycle rule is used to avoid import cycles.

Closes gh-4544
Ref gh-4541
Ref 0753201
mgol added a commit to mgol/jquery that referenced this pull request Nov 27, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull request Nov 27, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull request Nov 27, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull request Nov 30, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull request Nov 30, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the ES modules sources in `src/` to AMD in `amd/`.
mgol added a commit to mgol/jquery that referenced this pull request Nov 30, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the 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 similar
no-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 mgol mentioned this pull request Nov 30, 2019
2 tasks
mgol added a commit to mgol/jquery that referenced this pull request Nov 30, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the 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 similar
no-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 request Nov 30, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the 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 similar
no-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 request Nov 30, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the 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 similar
no-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 request Dec 2, 2019
jQuery source has been migrated in jquerygh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the 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 similar
no-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 request Dec 9, 2019
jQuery source has been migrated in gh-4541 from AMD to ES modules. To maintain
support for consumers of our AMD modules, this commits adds a task transpiling
the 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 similar
no-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).

Ref gh-4541
Closes gh-4554
mgol added a commit to mgol/jquery that referenced this pull request Feb 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 primitive
variable. This had to be changed on `master` as in ES modules you export
live read-only bindings to variables, meaning you can't increment the nonce
directly. Also, the way it was done so far was working differently in AMD & the
single built file - in the built file one nonce variable was declared, accessed
and incremented. In AMD mode separate instances were create for each module
that depend on the nonce module, creating unintended nonce clashes.
2. Whether the `noGlobal` parameter was set to `true` is now checked using the
typeof operator to align with `master`.

Ref jquerygh-4541
Ref d0ce00c
mgol added a commit that referenced this pull request Feb 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 primitive
variable. This had to be changed on `master` as in ES modules you export
live read-only bindings to variables, meaning you can't increment the nonce
directly. Also, the way it was done so far was working differently in AMD & the
single built file - in the built file one nonce variable was declared, accessed
and incremented. In AMD mode separate instances were create for each module
that depend on the nonce module, creating unintended nonce clashes.
2. Whether the `noGlobal` parameter was set to `true` is now checked using the
typeof operator to align with `master`.

Closes gh-4612
Ref gh-4541
Ref d0ce00c
@rart
Copy link

rart commented Apr 3, 2020

@mgol when will this effort be published to npm?

@mgol
Copy link
Member Author

mgol commented Apr 3, 2020

@rart This is planned for jQuery 4.0.0 which will take at least several more months. Migrating this to the 3.x-stable line while preserving backwards compatibility of the package structure might require non-trivial amount of work so at the moment we don't have plans to do it.

@rart
Copy link

rart commented Apr 4, 2020

@mgol thanks for the quick reply.

Have you guys considered an 4.0.0.rc sort of release/publish? (alpha, beta, rc)

@mgol
Copy link
Member Author

mgol commented Apr 4, 2020

@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 master, you can always copy our src directory. Just remember it's not stable so you'll need to be careful with upgrades.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment