Skip to content

Commit

Permalink
DependencyExtractionWebpackPlugin: Throw when using scripts from modu…
Browse files Browse the repository at this point in the history
…les (#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 <luisherranz@gmail.com>
  • Loading branch information
2 people authored and scruffian committed Jan 17, 2024
1 parent 79bb082 commit 9c69a6b
Show file tree
Hide file tree
Showing 17 changed files with 205 additions and 46 deletions.
50 changes: 30 additions & 20 deletions packages/dependency-extraction-webpack-plugin/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,38 @@ class DependencyExtractionWebpackPlugin {
externalizeWpDeps( { request }, callback ) {
let externalRequest;

// Handle via options.requestToExternal(Module) first.
if ( this.useModules ) {
if ( typeof this.options.requestToExternalModule === 'function' ) {
externalRequest =
this.options.requestToExternalModule( request );
try {
// Handle via options.requestToExternal(Module) first.
if ( this.useModules ) {
if (
typeof this.options.requestToExternalModule === 'function'
) {
externalRequest =
this.options.requestToExternalModule( request );

// requestToExternalModule allows a boolean shorthand
if ( externalRequest === false ) {
externalRequest = undefined;
}
if ( externalRequest === true ) {
externalRequest = request;
}
}
} else if ( typeof this.options.requestToExternal === 'function' ) {
externalRequest = this.options.requestToExternal( request );
}
} else if ( typeof this.options.requestToExternal === 'function' ) {
externalRequest = this.options.requestToExternal( request );
}

// Cascade to default if unhandled and enabled.
if (
typeof externalRequest === 'undefined' &&
this.options.useDefaults
) {
externalRequest = this.useModules
? defaultRequestToExternalModule( request )
: defaultRequestToExternal( request );
}

if ( this.useModules && externalRequest === true ) {
externalRequest = request;
// Cascade to default if unhandled and enabled.
if (
typeof externalRequest === 'undefined' &&
this.options.useDefaults
) {
externalRequest = this.useModules
? defaultRequestToExternalModule( request )
: defaultRequestToExternal( request );
}
} catch ( err ) {
return callback( err );
}

if ( externalRequest ) {
Expand Down
16 changes: 14 additions & 2 deletions packages/dependency-extraction-webpack-plugin/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,13 @@ function defaultRequestToExternal( request ) {
*
* Currently only @wordpress/interactivity
*
* Do not use the boolean shorthand here, it's only handled for the `requestToExternalModule` option.
*
* @param {string} request Module request (the module name in `import from`) to be transformed
* @return {string|undefined} The resulting external definition. Return `undefined`
* 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`.
* @return {string|Error|undefined} The resulting external definition.
* - Return `undefined` to ignore the request (do not externalize).
* - Return `string` to map the request to an external.
* - Return `Error` to emit an error.
*/
function defaultRequestToExternalModule( request ) {
if ( request === '@wordpress/interactivity' ) {
Expand All @@ -73,6 +77,14 @@ function defaultRequestToExternalModule( request ) {
// which forces @wordpress/interactivity imports to be hoisted to static imports.
return `module ${ request }`;
}

const isWordPressScript = Boolean( defaultRequestToExternal( request ) );

if ( isWordPressScript ) {
throw new Error(
`Attempted to use WordPress script in a module: ${ request }, which is not supported yet.`
);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`DependencyExtractionWebpackPlugin modules Webpack \`combine-assets\` should produce expected output: Asset file 'assets.php' should match snapshot 1`] = `
"<?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'));
"<?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'));
"
`;

Expand All @@ -17,6 +17,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`combine-assets\` sh
"request": "@wordpress/token-list",
"userRequest": "@wordpress/token-list",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

Expand Down Expand Up @@ -65,8 +70,17 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`dynamic-import\` sh
]
`;

exports[`DependencyExtractionWebpackPlugin modules Webpack \`error\` should produce expected output 1`] = `
"ERROR in ./index.js 5:0-23
Module not found: Error: Attempted to use WordPress script in a module: jquery, which is not supported yet.
ERROR in ./index.js 6:23-55
Module not found: Error: Attempted to use WordPress script in a module: @wordpress/api-fetch, which is not supported yet.
"
`;

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

Expand All @@ -77,11 +91,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`function-output-fil
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

Expand All @@ -92,6 +111,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`has-extension-suffi
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

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

Expand All @@ -141,11 +165,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`option-function-out
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

Expand All @@ -156,12 +185,25 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`option-output-filen
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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"}"`;
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"}"`;

exports[`DependencyExtractionWebpackPlugin modules Webpack \`output-format-json\` should produce expected output: External modules should match snapshot 1`] = `[]`;
exports[`DependencyExtractionWebpackPlugin modules Webpack \`output-format-json\` should produce expected output: External modules should match snapshot 1`] = `
[
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

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

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

Expand All @@ -215,11 +257,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`runtime-chunk-singl
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

Expand All @@ -230,11 +277,16 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`style-imports\` sho
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

Expand All @@ -245,16 +297,26 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress\` should
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

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

exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-interactivity\` should produce expected output: External modules should match snapshot 1`] = `
[
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
{
"externalType": "module",
"request": "@wordpress/interactivity",
Expand All @@ -264,7 +326,7 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-interacti
`;

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

Expand All @@ -275,6 +337,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-require\`
"request": "@wordpress/blob",
"userRequest": "@wordpress/blob",
},
{
"externalType": "import",
"request": "lodash",
"userRequest": "lodash",
},
]
`;

Expand Down Expand Up @@ -363,6 +430,12 @@ exports[`DependencyExtractionWebpackPlugin scripts Webpack \`dynamic-import\` sh
]
`;

exports[`DependencyExtractionWebpackPlugin scripts Webpack \`error\` should produce expected output 1`] = `
"ERROR in main
Module not found: Error: Ensure error in script build.
"
`;

exports[`DependencyExtractionWebpackPlugin scripts Webpack \`function-output-filename\` should produce expected output: Asset file 'chunk--main--main.asset.php' should match snapshot 1`] = `
"<?php return array('dependencies' => array('lodash', 'wp-blob'), 'version' => 'fc2d750fc9e08c5749db');
"
Expand Down
10 changes: 10 additions & 0 deletions packages/dependency-extraction-webpack-plugin/test/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ describe.each( /** @type {const} */ ( [ 'scripts', 'modules' ] ) )(
} )
);

/* eslint-disable jest/no-conditional-expect */
if ( configCase.includes( 'error' ) ) {
expect( stats.hasErrors() ).toBe( true );
expect(
stats.toString( { errors: true, all: false } )
).toMatchSnapshot();
return;
}
/* eslint-enable jest/no-conditional-expect */

if ( stats.hasErrors() ) {
throw new Error(
stats.toString( { errors: true, all: false } )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ module.exports = {
new DependencyExtractionWebpackPlugin( {
combineAssets: true,
requestToExternalModule( request ) {
return request.startsWith( '@wordpress/' );
return (
request.startsWith( '@wordpress/' ) || request === 'lodash'
);
},
} ),
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/* eslint-disable eslint-comments/disable-enable-pair */
/* eslint-disable eslint-comments/no-unlimited-disable */
/* eslint-disable */

import $ from 'jquery';
const apiFetch = await import( '@wordpress/api-fetch' );

$( () => {
apiFetch( { path: '/' } );
} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Internal dependencies
*/
const DependencyExtractionWebpackPlugin = require( '../../..' );

module.exports = {
plugins: [
new DependencyExtractionWebpackPlugin( {
// eslint-disable-next-line no-unused-vars
requestToExternal( request ) {
throw new Error( 'Ensure error in script build.' );
},
} ),
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ module.exports = {
plugins: [
new DependencyExtractionWebpackPlugin( {
requestToExternalModule( request ) {
return request.startsWith( '@wordpress/' );
return (
request.startsWith( '@wordpress/' ) || request === 'lodash'
);
},
} ),
],
Expand Down
Loading

0 comments on commit 9c69a6b

Please sign in to comment.