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
@sirreal @luisherranz
sirreal and luisherranz committed Jan 12, 2024
1 parent 55b4e9b commit 858bc3b
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
View file
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
View file
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
99 changes: 86 additions & 13 deletions packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap
View file
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
View file
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
4 changes: 3 additions & 1 deletion packages/dependency-extraction-webpack-plugin/test/fixtures/combine-assets/webpack.config.js
View file
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
10 changes: 10 additions & 0 deletions packages/dependency-extraction-webpack-plugin/test/fixtures/error/index.js
View file
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: '/' } );
} );
15 changes: 15 additions & 0 deletions packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js
View file
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.' );
},
} ),
],
};
4 changes: 3 additions & 1 deletion ...ndency-extraction-webpack-plugin/test/fixtures/function-output-filename/webpack.config.js
View file
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

1 comment on commit 858bc3b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 858bc3b.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7504360965
📝 Reported issues:

Please sign in to comment.