Skip to content

Commit 56e8f5f

Browse files
sirrealluisherranz
andcommitted
DependencyExtractionWebpackPlugin: Throw when using scripts from modules (#57795)
WordPress Script dependencies are not currently available as dependencies of WordPress Modules. Using e.g. lodash or @wordpress/api-fetch in a module build would result in bundling that dependency. For a package like lodash that's undesirable but should work. However, many @wordpress/* packages are not intended to be bundle or duplicated and will not work as expected. It's likely an error to use WordPress Scripts inside modules at this time. --------- Co-authored-by: Luis Herranz <[email protected]>
1 parent beff93c commit 56e8f5f

File tree

17 files changed

+205
-46
lines changed

17 files changed

+205
-46
lines changed

packages/dependency-extraction-webpack-plugin/lib/index.js

+30-20
Original file line numberDiff line numberDiff line change
@@ -62,28 +62,38 @@ class DependencyExtractionWebpackPlugin {
6262
externalizeWpDeps( { request }, callback ) {
6363
let externalRequest;
6464

65-
// Handle via options.requestToExternal(Module) first.
66-
if ( this.useModules ) {
67-
if ( typeof this.options.requestToExternalModule === 'function' ) {
68-
externalRequest =
69-
this.options.requestToExternalModule( request );
65+
try {
66+
// Handle via options.requestToExternal(Module) first.
67+
if ( this.useModules ) {
68+
if (
69+
typeof this.options.requestToExternalModule === 'function'
70+
) {
71+
externalRequest =
72+
this.options.requestToExternalModule( request );
73+
74+
// requestToExternalModule allows a boolean shorthand
75+
if ( externalRequest === false ) {
76+
externalRequest = undefined;
77+
}
78+
if ( externalRequest === true ) {
79+
externalRequest = request;
80+
}
81+
}
82+
} else if ( typeof this.options.requestToExternal === 'function' ) {
83+
externalRequest = this.options.requestToExternal( request );
7084
}
71-
} else if ( typeof this.options.requestToExternal === 'function' ) {
72-
externalRequest = this.options.requestToExternal( request );
73-
}
74-
75-
// Cascade to default if unhandled and enabled.
76-
if (
77-
typeof externalRequest === 'undefined' &&
78-
this.options.useDefaults
79-
) {
80-
externalRequest = this.useModules
81-
? defaultRequestToExternalModule( request )
82-
: defaultRequestToExternal( request );
83-
}
8485

85-
if ( this.useModules && externalRequest === true ) {
86-
externalRequest = request;
86+
// Cascade to default if unhandled and enabled.
87+
if (
88+
typeof externalRequest === 'undefined' &&
89+
this.options.useDefaults
90+
) {
91+
externalRequest = this.useModules
92+
? defaultRequestToExternalModule( request )
93+
: defaultRequestToExternal( request );
94+
}
95+
} catch ( err ) {
96+
return callback( err );
8797
}
8898

8999
if ( externalRequest ) {

packages/dependency-extraction-webpack-plugin/lib/util.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,13 @@ function defaultRequestToExternal( request ) {
6161
*
6262
* Currently only @wordpress/interactivity
6363
*
64+
* Do not use the boolean shorthand here, it's only handled for the `requestToExternalModule` option.
65+
*
6466
* @param {string} request Module request (the module name in `import from`) to be transformed
65-
* @return {string|undefined} The resulting external definition. Return `undefined`
66-
* to ignore the request. Return `string` to map the request to an external. This may simply be returning the request, e.g. `@wordpress/interactivity` maps to the external `@wordpress/interactivity`.
67+
* @return {string|Error|undefined} The resulting external definition.
68+
* - Return `undefined` to ignore the request (do not externalize).
69+
* - Return `string` to map the request to an external.
70+
* - Return `Error` to emit an error.
6771
*/
6872
function defaultRequestToExternalModule( request ) {
6973
if ( request === '@wordpress/interactivity' ) {
@@ -73,6 +77,14 @@ function defaultRequestToExternalModule( request ) {
7377
// which forces @wordpress/interactivity imports to be hoisted to static imports.
7478
return `module ${ request }`;
7579
}
80+
81+
const isWordPressScript = Boolean( defaultRequestToExternal( request ) );
82+
83+
if ( isWordPressScript ) {
84+
throw new Error(
85+
`Attempted to use WordPress script in a module: ${ request }, which is not supported yet.`
86+
);
87+
}
7688
}
7789

7890
/**

packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap

+86-13
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`DependencyExtractionWebpackPlugin modules Webpack \`combine-assets\` should produce expected output: Asset file 'assets.php' should match snapshot 1`] = `
4-
"<?php return array('fileA.mjs' => array('dependencies' => array('@wordpress/blob'), 'version' => 'b2c5cea79a32b3d91bf8', 'type' => 'module'), 'fileB.mjs' => array('dependencies' => array('@wordpress/token-list'), 'version' => '5c4197fd48811f25807f', 'type' => 'module'));
4+
"<?php return array('fileA.mjs' => array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => '4ab8cc4b6b7619053443', 'type' => 'module'), 'fileB.mjs' => array('dependencies' => array('@wordpress/token-list'), 'version' => '5c4197fd48811f25807f', 'type' => 'module'));
55
"
66
`;
77

@@ -17,6 +17,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`combine-assets\` sh
1717
"request": "@wordpress/token-list",
1818
"userRequest": "@wordpress/token-list",
1919
},
20+
{
21+
"externalType": "import",
22+
"request": "lodash",
23+
"userRequest": "lodash",
24+
},
2025
]
2126
`;
2227

@@ -65,8 +70,17 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`dynamic-import\` sh
6570
]
6671
`;
6772

73+
exports[`DependencyExtractionWebpackPlugin modules Webpack \`error\` should produce expected output 1`] = `
74+
"ERROR in ./index.js 5:0-23
75+
Module not found: Error: Attempted to use WordPress script in a module: jquery, which is not supported yet.
76+
77+
ERROR in ./index.js 6:23-55
78+
Module not found: Error: Attempted to use WordPress script in a module: @wordpress/api-fetch, which is not supported yet.
79+
"
80+
`;
81+
6882
exports[`DependencyExtractionWebpackPlugin modules Webpack \`function-output-filename\` should produce expected output: Asset file 'chunk--main--main.asset.php' should match snapshot 1`] = `
69-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module');
83+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module');
7084
"
7185
`;
7286

@@ -77,11 +91,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`function-output-fil
7791
"request": "@wordpress/blob",
7892
"userRequest": "@wordpress/blob",
7993
},
94+
{
95+
"externalType": "import",
96+
"request": "lodash",
97+
"userRequest": "lodash",
98+
},
8099
]
81100
`;
82101

83102
exports[`DependencyExtractionWebpackPlugin modules Webpack \`has-extension-suffix\` should produce expected output: Asset file 'index.min.asset.php' should match snapshot 1`] = `
84-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '26d6da43027f3522b0ca', 'type' => 'module');
103+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => '8308f1ac78c21f09c721', 'type' => 'module');
85104
"
86105
`;
87106

@@ -92,6 +111,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`has-extension-suffi
92111
"request": "@wordpress/blob",
93112
"userRequest": "@wordpress/blob",
94113
},
114+
{
115+
"externalType": "import",
116+
"request": "lodash",
117+
"userRequest": "lodash",
118+
},
95119
]
96120
`;
97121

@@ -130,7 +154,7 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`no-deps\` should pr
130154
exports[`DependencyExtractionWebpackPlugin modules Webpack \`no-deps\` should produce expected output: External modules should match snapshot 1`] = `[]`;
131155

132156
exports[`DependencyExtractionWebpackPlugin modules Webpack \`option-function-output-filename\` should produce expected output: Asset file 'chunk--main--main.asset.php' should match snapshot 1`] = `
133-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module');
157+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module');
134158
"
135159
`;
136160

@@ -141,11 +165,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`option-function-out
141165
"request": "@wordpress/blob",
142166
"userRequest": "@wordpress/blob",
143167
},
168+
{
169+
"externalType": "import",
170+
"request": "lodash",
171+
"userRequest": "lodash",
172+
},
144173
]
145174
`;
146175

147176
exports[`DependencyExtractionWebpackPlugin modules Webpack \`option-output-filename\` should produce expected output: Asset file 'main-foo.asset.php' should match snapshot 1`] = `
148-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module');
177+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module');
149178
"
150179
`;
151180

@@ -156,12 +185,25 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`option-output-filen
156185
"request": "@wordpress/blob",
157186
"userRequest": "@wordpress/blob",
158187
},
188+
{
189+
"externalType": "import",
190+
"request": "lodash",
191+
"userRequest": "lodash",
192+
},
159193
]
160194
`;
161195

162-
exports[`DependencyExtractionWebpackPlugin modules Webpack \`output-format-json\` should produce expected output: Asset file 'main.asset.json' should match snapshot 1`] = `"{"dependencies":[],"version":"34504aa793c63cd3d73a","type":"module"}"`;
196+
exports[`DependencyExtractionWebpackPlugin modules Webpack \`output-format-json\` should produce expected output: Asset file 'main.asset.json' should match snapshot 1`] = `"{"dependencies":["lodash"],"version":"4e62c01516f9dab8041f","type":"module"}"`;
163197

164-
exports[`DependencyExtractionWebpackPlugin modules Webpack \`output-format-json\` should produce expected output: External modules should match snapshot 1`] = `[]`;
198+
exports[`DependencyExtractionWebpackPlugin modules Webpack \`output-format-json\` should produce expected output: External modules should match snapshot 1`] = `
199+
[
200+
{
201+
"externalType": "import",
202+
"request": "lodash",
203+
"userRequest": "lodash",
204+
},
205+
]
206+
`;
165207

166208
exports[`DependencyExtractionWebpackPlugin modules Webpack \`overrides\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
167209
"<?php return array('dependencies' => array('@wordpress/blob', '@wordpress/url', 'rxjs', 'rxjs/operators'), 'version' => '259fc706528651fc00c1', 'type' => 'module');
@@ -199,12 +241,12 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`runtime-chunk-singl
199241
`;
200242

201243
exports[`DependencyExtractionWebpackPlugin modules Webpack \`runtime-chunk-single\` should produce expected output: Asset file 'b.asset.php' should match snapshot 1`] = `
202-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => 'a0ec8ef279476bb51e19', 'type' => 'module');
244+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => 'e14dfa7260edaee86a85', 'type' => 'module');
203245
"
204246
`;
205247

206248
exports[`DependencyExtractionWebpackPlugin modules Webpack \`runtime-chunk-single\` should produce expected output: Asset file 'runtime.asset.php' should match snapshot 1`] = `
207-
"<?php return array('dependencies' => array(), 'version' => '0bb8a9fae3dcfcc1ac38', 'type' => 'module');
249+
"<?php return array('dependencies' => array(), 'version' => 'e7402f5608a888d8fd66', 'type' => 'module');
208250
"
209251
`;
210252

@@ -215,11 +257,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`runtime-chunk-singl
215257
"request": "@wordpress/blob",
216258
"userRequest": "@wordpress/blob",
217259
},
260+
{
261+
"externalType": "import",
262+
"request": "lodash",
263+
"userRequest": "lodash",
264+
},
218265
]
219266
`;
220267

221268
exports[`DependencyExtractionWebpackPlugin modules Webpack \`style-imports\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
222-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '38479966fb62d588f05e', 'type' => 'module');
269+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => '22d18a3461df47fbaa79', 'type' => 'module');
223270
"
224271
`;
225272

@@ -230,11 +277,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`style-imports\` sho
230277
"request": "@wordpress/blob",
231278
"userRequest": "@wordpress/blob",
232279
},
280+
{
281+
"externalType": "import",
282+
"request": "lodash",
283+
"userRequest": "lodash",
284+
},
233285
]
234286
`;
235287

236288
exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
237-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module');
289+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module');
238290
"
239291
`;
240292

@@ -245,16 +297,26 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress\` should
245297
"request": "@wordpress/blob",
246298
"userRequest": "@wordpress/blob",
247299
},
300+
{
301+
"externalType": "import",
302+
"request": "lodash",
303+
"userRequest": "lodash",
304+
},
248305
]
249306
`;
250307

251308
exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-interactivity\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
252-
"<?php return array('dependencies' => array(array('id' => '@wordpress/interactivity', 'type' => 'dynamic')), 'version' => 'f0242eb6da78af6ca4b8', 'type' => 'module');
309+
"<?php return array('dependencies' => array('lodash', array('id' => '@wordpress/interactivity', 'type' => 'dynamic')), 'version' => 'fcc07ce68574cdc2a6a5', 'type' => 'module');
253310
"
254311
`;
255312

256313
exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-interactivity\` should produce expected output: External modules should match snapshot 1`] = `
257314
[
315+
{
316+
"externalType": "import",
317+
"request": "lodash",
318+
"userRequest": "lodash",
319+
},
258320
{
259321
"externalType": "module",
260322
"request": "@wordpress/interactivity",
@@ -264,7 +326,7 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-interacti
264326
`;
265327

266328
exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-require\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
267-
"<?php return array('dependencies' => array('@wordpress/blob'), 'version' => '52c1849898b74d94f025', 'type' => 'module');
329+
"<?php return array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => 'f40de15d66b54da440d2', 'type' => 'module');
268330
"
269331
`;
270332

@@ -275,6 +337,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-require\`
275337
"request": "@wordpress/blob",
276338
"userRequest": "@wordpress/blob",
277339
},
340+
{
341+
"externalType": "import",
342+
"request": "lodash",
343+
"userRequest": "lodash",
344+
},
278345
]
279346
`;
280347

@@ -363,6 +430,12 @@ exports[`DependencyExtractionWebpackPlugin scripts Webpack \`dynamic-import\` sh
363430
]
364431
`;
365432

433+
exports[`DependencyExtractionWebpackPlugin scripts Webpack \`error\` should produce expected output 1`] = `
434+
"ERROR in main
435+
Module not found: Error: Ensure error in script build.
436+
"
437+
`;
438+
366439
exports[`DependencyExtractionWebpackPlugin scripts Webpack \`function-output-filename\` should produce expected output: Asset file 'chunk--main--main.asset.php' should match snapshot 1`] = `
367440
"<?php return array('dependencies' => array('lodash', 'wp-blob'), 'version' => 'fc2d750fc9e08c5749db');
368441
"

packages/dependency-extraction-webpack-plugin/test/build.js

+10
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,16 @@ describe.each( /** @type {const} */ ( [ 'scripts', 'modules' ] ) )(
7272
} )
7373
);
7474

75+
/* eslint-disable jest/no-conditional-expect */
76+
if ( configCase.includes( 'error' ) ) {
77+
expect( stats.hasErrors() ).toBe( true );
78+
expect(
79+
stats.toString( { errors: true, all: false } )
80+
).toMatchSnapshot();
81+
return;
82+
}
83+
/* eslint-enable jest/no-conditional-expect */
84+
7585
if ( stats.hasErrors() ) {
7686
throw new Error(
7787
stats.toString( { errors: true, all: false } )

packages/dependency-extraction-webpack-plugin/test/fixtures/combine-assets/webpack.config.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ module.exports = {
1212
new DependencyExtractionWebpackPlugin( {
1313
combineAssets: true,
1414
requestToExternalModule( request ) {
15-
return request.startsWith( '@wordpress/' );
15+
return (
16+
request.startsWith( '@wordpress/' ) || request === 'lodash'
17+
);
1618
},
1719
} ),
1820
],
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/* eslint-disable eslint-comments/disable-enable-pair */
2+
/* eslint-disable eslint-comments/no-unlimited-disable */
3+
/* eslint-disable */
4+
5+
import $ from 'jquery';
6+
const apiFetch = await import( '@wordpress/api-fetch' );
7+
8+
$( () => {
9+
apiFetch( { path: '/' } );
10+
} );
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* Internal dependencies
3+
*/
4+
const DependencyExtractionWebpackPlugin = require( '../../..' );
5+
6+
module.exports = {
7+
plugins: [
8+
new DependencyExtractionWebpackPlugin( {
9+
// eslint-disable-next-line no-unused-vars
10+
requestToExternal( request ) {
11+
throw new Error( 'Ensure error in script build.' );
12+
},
13+
} ),
14+
],
15+
};

packages/dependency-extraction-webpack-plugin/test/fixtures/function-output-filename/webpack.config.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ module.exports = {
1212
plugins: [
1313
new DependencyExtractionWebpackPlugin( {
1414
requestToExternalModule( request ) {
15-
return request.startsWith( '@wordpress/' );
15+
return (
16+
request.startsWith( '@wordpress/' ) || request === 'lodash'
17+
);
1618
},
1719
} ),
1820
],

0 commit comments

Comments
 (0)