Skip to content

Commit 60f11b5

Browse files
mgolmgol
authored
Core: Fix the exports setup to make bundlers work with ESM & CommonJS
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. As for the factory entry points: 1. Node.js always gets the CommonJS version 2. Bundlers always get the ESM version 3. Other tools take the ESM version when using `import` and the CommonJS when using `require`. The complexity is lower than for the `.` & `./slim` entry points because there's no default export to handle so Node/bundler wrapper files are not necessary. Other changes: * Tests: Change "node:assert" to "node:assert/strict"; the former is deprecated * Docs: Mention that the CommonJS module doesn't expose named exports * Tests: Run Node & bundler tests for all the above cases Fixes gh-5416 Closes gh-5429
1 parent fedffe7 commit 60f11b5

35 files changed

+1173
-48
lines changed

‎.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,16 @@
99
.sizecache.json
1010
yarn.lock
1111
.eslintcache
12+
tmp
1213

1314
npm-debug.log*
1415

1516
# Ignore everything in `dist` folder except for
1617
# the ESLint config & package.json files
1718
/dist/*
1819
!/dist/package.json
20+
!/dist/jquery.bundler-require-wrapper.js
21+
!/dist/jquery.bundler-require-wrapper.slim.js
1922

2023
# Ignore everything in the `dist-module` folder except for the ESLint config,
2124
# package.json & Node module wrapper files

‎build/fixtures/README.md

+2
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ If you need to use jQuery in a file that's not an ECMAScript module, you can use
120120
const $ = require( "jquery" );
121121
```
122122

123+
The CommonJS module _does not_ expose named `$` & `jQuery` exports.
124+
123125
#### Individual modules
124126

125127
jQuery is authored in ECMAScript modules; it's also possible to use them directly. They are contained in the `src/` folder; inspect the package contents to see what's there. Full file names are required, including the `.js` extension.

‎build/release/dist.js

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ module.exports = function( Release, files, complete ) {
1717
"LICENSE.txt",
1818
"AUTHORS.txt",
1919
"dist/package.json",
20+
"dist/jquery.bundler-require-wrapper.js",
21+
"dist/jquery.bundler-require-wrapper.slim.js",
2022
"dist-module/package.json",
2123
"dist-module/jquery.node-module-wrapper.js",
2224
"dist-module/jquery.node-module-wrapper.slim.js"

‎build/tasks/node_smoke_tests.js

+26-4
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ const util = require( "node:util" );
55
const exec = util.promisify( require( "node:child_process" ).exec );
66
const verifyNodeVersion = require( "./lib/verifyNodeVersion" );
77

8-
const allowedLibraryTypes = [ "regular", "factory" ];
9-
const allowedSourceTypes = [ "commonjs", "module" ];
8+
const allowedLibraryTypes = new Set( [ "regular", "factory" ] );
9+
const allowedSourceTypes = new Set( [ "commonjs", "module", "dual" ] );
1010

1111
if ( !verifyNodeVersion() ) {
1212
return;
@@ -19,8 +19,8 @@ if ( !verifyNodeVersion() ) {
1919
// each other, e.g. so that they don't share the `require` cache.
2020

2121
async function runTests( { libraryType, sourceType, module } ) {
22-
if ( !allowedLibraryTypes.includes( libraryType ) ||
23-
!allowedSourceTypes.includes( sourceType ) ) {
22+
if ( !allowedLibraryTypes.has( libraryType ) ||
23+
!allowedSourceTypes.has( sourceType ) ) {
2424
throw new Error( `Incorrect libraryType or sourceType value; passed: ${
2525
libraryType
2626
} ${ sourceType } "${ module }"` );
@@ -127,6 +127,28 @@ async function runDefaultTests() {
127127
libraryType: "factory",
128128
sourceType: "module",
129129
module: "./dist-module/jquery.factory.slim.module.js"
130+
} ),
131+
132+
runTests( {
133+
libraryType: "regular",
134+
sourceType: "dual",
135+
module: "jquery"
136+
} ),
137+
runTests( {
138+
libraryType: "regular",
139+
sourceType: "dual",
140+
module: "jquery/slim"
141+
} ),
142+
143+
runTests( {
144+
libraryType: "factory",
145+
sourceType: "dual",
146+
module: "jquery/factory"
147+
} ),
148+
runTests( {
149+
libraryType: "factory",
150+
sourceType: "dual",
151+
module: "jquery/factory-slim"
130152
} )
131153
] );
132154
}

‎dist-module/jquery.node-module-wrapper.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1+
// Node.js is able to import from a CommonJS module in an ESM one.
12
import jQuery from "../dist/jquery.js";
2-
const $ = jQuery;
3-
export { jQuery, $ };
3+
4+
export { jQuery, jQuery as $ };
45
export default jQuery;

‎dist-module/jquery.node-module-wrapper.slim.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1+
// Node.js is able to import from a CommonJS module in an ESM one.
12
import jQuery from "../dist/jquery.slim.js";
2-
const $ = jQuery;
3-
export { jQuery, $ };
3+
4+
export { jQuery, jQuery as $ };
45
export default jQuery;

‎dist/jquery.bundler-require-wrapper.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"use strict";
2+
3+
// Bundlers are able to synchronously require an ESM module from a CommonJS one.
4+
const { jQuery } = require( "../dist-module/jquery.module.js" );
5+
module.exports = jQuery;

‎dist/jquery.bundler-require-wrapper.slim.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"use strict";
2+
3+
// Bundlers are able to synchronously require an ESM module from a CommonJS one.
4+
const { jQuery } = require( "../dist-module/jquery.slim.module.js" );
5+
module.exports = jQuery;

‎eslint.config.js

+24-5
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export default [
1010
// See https://github.com/eslint/eslint/discussions/17412
1111
ignores: [
1212
"external",
13+
"**/tmp",
1314
"test/data/json_obj.js"
1415
]
1516
},
@@ -18,8 +19,8 @@ export default [
1819
files: [
1920
"eslint.config.js",
2021
"Gruntfile.cjs",
21-
"test/node_smoke_tests/commonjs/**",
22-
"test/node_smoke_tests/module/**",
22+
"test/bundler_smoke_tests/**/*",
23+
"test/node_smoke_tests/**",
2324
"test/promises_aplus_adapters/**",
2425
"test/middleware-mockserver.cjs"
2526
],
@@ -260,8 +261,8 @@ export default [
260261

261262
{
262263
files: [
263-
"test/node_smoke_tests/commonjs/**",
264-
"test/node_smoke_tests/module/**",
264+
"test/bundler_smoke_tests/**",
265+
"test/node_smoke_tests/**",
265266
"test/promises_aplus_adapters/**",
266267
"test/middleware-mockserver.cjs"
267268
],
@@ -317,7 +318,25 @@ export default [
317318

318319
languageOptions: {
319320
globals: {
320-
...globals.browser,
321+
...globals.es2021,
322+
define: false,
323+
module: false,
324+
Symbol: false
325+
}
326+
}
327+
},
328+
329+
{
330+
files: [
331+
"dist/jquery.bundler-require-wrapper.js",
332+
"dist/jquery.bundler-require-wrapper.slim.js",
333+
"dist-module/jquery.node-module-wrapper.js",
334+
"dist-module/jquery.node-module-wrapper.slim.js"
335+
],
336+
337+
languageOptions: {
338+
globals: {
339+
...globals.node,
321340
...globals.es2021,
322341
define: false,
323342
module: false,

0 commit comments

Comments
 (0)