jquery / jquery

jQuery JavaScript Library
https://jquery.com
MIT License
58.93k stars 20.62k forks source link

Core: Fix the exports setup to make bundlers work with ESM & CommonJS #5429

Closed mgol closed 2 months ago

mgol commented 2 months ago

Summary

We cannot pass a single file via the module condition as then require( "jquery" ) will not return jQuery but instead the module object with default, $ & jQuery as keys. Instead:

  1. For Node.js, detected via the node condition:
    1. Expose a regular CommonJS version to require
    2. Expose a tiny wrapper over CommonJS to import
  2. For bundlers, detected via the module condition:
    1. Expose a regular ESM version to import
    2. Expose a tiny wrapper over ESM to require
  3. If neither Node.js nor bundlers are detected (no node or module conditions`):
    1. Expose a regular CommonJS version to require
    2. Expose a regular ESM version to import

The reasons for such definitions are as follows:

  1. In Node.js, one can synchronously import from a CommonJS file inside of an ESM one but not vice-versa. To use an ESM file in a CommonJS one, a dynamic import is required and that forces asynchronicity.
  2. In some bundlers CommonJS is not necessarily enabled - e.g. in Rollup without the CommonJS plugin. Therefore, the ESM version needs to be pure ESM. However, bundlers allow synchronously calling require on an ESM file. This is possible since bundlers merge the files before they are passed to the browser to execute and the final bundles no longer contain async import code.
  3. Bare ESM & CommonJS versions are provided to non-Node non-bundler environments where we cannot assume interoperability between ESM & CommonJS is supported.
  4. Bare versions cannot be supplied to Node or bundlers as projects using both ESM & CommonJS to fetch jQuery would result in duplicate jQuery instances, leading to increased JS size and disjoint data storage.

In addition to the above changes, the script condition has been dropped. Only Webpack documents this condition and it's not clear when exactly it's triggered. Adding support for a new condition can be added later without a breaking change; removing is not so easy.

The production & development conditions have been removed as well. They were not really applied correctly; we'd need to provide both of them to each current leaf which would double the size of the definition for the . & ./slim entry points. In jQuery, the only difference between development & production builds is minification; there are no logic changes so we can pass unminified versions to all the tooling, expecting minification down the line.

For ./factory & ./factory-slim entry points, only the ESM version is now supported. This was done to simplify the setup for this niche use case.

Tests with Rollup (both pure ESM & with the CommonJS plugin) & Webpack have been added.

I added an explainer document to the wiki at https://github.com/jquery/jquery/wiki/jQuery-4-exports-explainer that should help understand the whole exports definition. It can be helpful especially if we need to make more changes.

Fixes gh-5416

Checklist

mgol commented 2 months ago

CI is failing because pretest now de facto depends on build:all. This means core tests now require a build. I need to think how to approach this.

mgol commented 2 months ago

@timmywil the PR is ready for review now.

timmywil commented 2 months ago

Very nice! I only have suggestions on the npm scripts

mgol commented 2 months ago

@timmywil I haven't addressed your feedback yet but I pushed 3 new commits that I had in progress (the first one is titled "Docs: Mention that the CommonJS module doesn't expose named exports"). The first one is a small docs update, the other two are adding more bundler/Node tests for our exports setup - for the slim build & both factory ones.

I contemplated simplifying the factory definitions by only exposing the ESM versions but I ultimately decided it's too breaking. People depending on factory behavior can update to our new entry points but they may not be able to switch to ESM. Funny enough, I hit it myself here as our Promises/A+ tests adapter uses the factory build but it's a CommonJS file and needs to expose the API synchronously...

The factory setup is still way simpler because there is no default export in ESM and the CommonJS version exports a similar object instead of just the API - and this means adapters are not needed, we can just expose the CommonJS version to Node and the ESM one to bundlers.

mgol commented 2 months ago

@timmywil I addressed your feedback now, please re-review.

mgol commented 2 months ago

@timmywil FYI, I added an explainer document to the wiki at https://github.com/jquery/jquery/wiki/jQuery-4-exports-explainer that should help understand the whole exports definition. It can be helpful especially if we need to make more changes.