From 4ba46324f093e456da6ed12997579129d07f27ef Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 12 Jan 2024 10:41:55 +0100 Subject: [PATCH 1/6] Implement error throwing on Scripts imported from Modules --- .../lib/index.js | 9 +++++---- .../lib/util.js | 15 +++++++++++++-- .../test/build.js | 10 ++++++++++ .../fixtures/combine-assets/webpack.config.js | 4 +++- .../test/fixtures/error/index.js | 10 ++++++++++ .../test/fixtures/error/webpack.config.js | 15 +++++++++++++++ .../function-output-filename/webpack.config.js | 4 +++- .../has-extension-suffix/webpack.config.js | 4 +++- .../webpack.config.js | 4 +++- .../option-output-filename/webpack.config.js | 4 +++- .../fixtures/output-format-json/webpack.config.js | 7 ++++++- .../runtime-chunk-single/webpack.config.js | 4 +++- .../test/fixtures/style-imports/webpack.config.js | 4 +++- .../wordpress-interactivity/webpack.config.js | 8 +++++++- .../fixtures/wordpress-require/webpack.config.js | 4 +++- .../test/fixtures/wordpress/webpack.config.js | 4 +++- 16 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 packages/dependency-extraction-webpack-plugin/test/fixtures/error/index.js create mode 100644 packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js diff --git a/packages/dependency-extraction-webpack-plugin/lib/index.js b/packages/dependency-extraction-webpack-plugin/lib/index.js index e8d9fc9bfee1d8..ea078f34a65a1c 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/index.js +++ b/packages/dependency-extraction-webpack-plugin/lib/index.js @@ -73,10 +73,7 @@ class DependencyExtractionWebpackPlugin { } // Cascade to default if unhandled and enabled. - if ( - typeof externalRequest === 'undefined' && - this.options.useDefaults - ) { + if ( ! externalRequest && this.options.useDefaults ) { externalRequest = this.useModules ? defaultRequestToExternalModule( request ) : defaultRequestToExternal( request ); @@ -86,6 +83,10 @@ class DependencyExtractionWebpackPlugin { externalRequest = request; } + if ( externalRequest instanceof Error ) { + return callback( externalRequest ); + } + if ( externalRequest ) { this.externalizedDeps.add( request ); diff --git a/packages/dependency-extraction-webpack-plugin/lib/util.js b/packages/dependency-extraction-webpack-plugin/lib/util.js index 50b7eaa8db98fb..1f314ea9eb28b5 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/util.js +++ b/packages/dependency-extraction-webpack-plugin/lib/util.js @@ -62,8 +62,11 @@ function defaultRequestToExternal( request ) { * Currently only @wordpress/interactivity * * @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|boolean|Error|undefined} The resulting external definition. + * - Return `undefined` to ignore the request (do not externalize). + * - Return `true` to externalize the request with the same Module ID + * - Return `string` to map the request to an external. + * - Return `Error` to emit an error. */ function defaultRequestToExternalModule( request ) { if ( request === '@wordpress/interactivity' ) { @@ -73,6 +76,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 ) { + return new Error( + `Attempted to use WordPress script in a module: ${ request }` + ); + } } /** diff --git a/packages/dependency-extraction-webpack-plugin/test/build.js b/packages/dependency-extraction-webpack-plugin/test/build.js index 3b29d55caf2bb0..0b28b679bba920 100644 --- a/packages/dependency-extraction-webpack-plugin/test/build.js +++ b/packages/dependency-extraction-webpack-plugin/test/build.js @@ -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 } ) diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/combine-assets/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/combine-assets/webpack.config.js index 420d5030ab1a69..ab407815bbee48 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/combine-assets/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/combine-assets/webpack.config.js @@ -12,7 +12,9 @@ module.exports = { new DependencyExtractionWebpackPlugin( { combineAssets: true, requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/error/index.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/error/index.js new file mode 100644 index 00000000000000..388963c34de897 --- /dev/null +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/error/index.js @@ -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: '/' } ); +} ); diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js new file mode 100644 index 00000000000000..b3583f7b0a1e6f --- /dev/null +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js @@ -0,0 +1,15 @@ +/** + * Internal dependencies + */ +const DependencyExtractionWebpackPlugin = require( '../../..' ); + +module.exports = { + plugins: [ + new DependencyExtractionWebpackPlugin( { + // eslint-disable-next-line no-unused-vars + requestToExternal( request ) { + return new Error( 'Ensure error in script build.' ); + }, + } ), + ], +}; diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/function-output-filename/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/function-output-filename/webpack.config.js index 0aea733327a540..7ec1c7ce8a5de1 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/function-output-filename/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/function-output-filename/webpack.config.js @@ -12,7 +12,9 @@ module.exports = { plugins: [ new DependencyExtractionWebpackPlugin( { requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/has-extension-suffix/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/has-extension-suffix/webpack.config.js index 4ef05f6986b9c4..4816746976c5b6 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/has-extension-suffix/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/has-extension-suffix/webpack.config.js @@ -10,7 +10,9 @@ module.exports = { plugins: [ new DependencyExtractionWebpackPlugin( { requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/option-function-output-filename/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/option-function-output-filename/webpack.config.js index 40123021ae404c..f65d51ffa5152f 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/option-function-output-filename/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/option-function-output-filename/webpack.config.js @@ -10,7 +10,9 @@ module.exports = { return `chunk--${ chunkData.chunk.name }--[name].asset.php`; }, requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/option-output-filename/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/option-output-filename/webpack.config.js index fb02bc3c5bcd0b..30aa1f0233e93e 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/option-output-filename/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/option-output-filename/webpack.config.js @@ -8,7 +8,9 @@ module.exports = { new DependencyExtractionWebpackPlugin( { outputFilename: '[name]-foo.asset.php', requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/output-format-json/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/output-format-json/webpack.config.js index b82f422f949f81..3a6099354c86bb 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/output-format-json/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/output-format-json/webpack.config.js @@ -5,6 +5,11 @@ const DependencyExtractionWebpackPlugin = require( '../../..' ); module.exports = { plugins: [ - new DependencyExtractionWebpackPlugin( { outputFormat: 'json' } ), + new DependencyExtractionWebpackPlugin( { + outputFormat: 'json', + requestToExternalModule( request ) { + return request === 'lodash'; + }, + } ), ], }; diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/runtime-chunk-single/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/runtime-chunk-single/webpack.config.js index b8fa673995e9be..0c6608a814b45e 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/runtime-chunk-single/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/runtime-chunk-single/webpack.config.js @@ -11,7 +11,9 @@ module.exports = { plugins: [ new DependencyExtractionWebpackPlugin( { requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/style-imports/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/style-imports/webpack.config.js index bb412af5c61b89..0f5ac2b955538a 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/style-imports/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/style-imports/webpack.config.js @@ -12,7 +12,9 @@ module.exports = { plugins: [ new DependencyExtractionWebpackPlugin( { requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), new MiniCSSExtractPlugin(), diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-interactivity/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-interactivity/webpack.config.js index bfffff3ae78319..7d18e80a8d4ea3 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-interactivity/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-interactivity/webpack.config.js @@ -4,5 +4,11 @@ const DependencyExtractionWebpackPlugin = require( '../../..' ); module.exports = { - plugins: [ new DependencyExtractionWebpackPlugin() ], + plugins: [ + new DependencyExtractionWebpackPlugin( { + requestToExternalModule( request ) { + return request === 'lodash'; + }, + } ), + ], }; diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-require/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-require/webpack.config.js index 59d4c5d2ead3ab..bf6cb387dbd18f 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-require/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress-require/webpack.config.js @@ -7,7 +7,9 @@ module.exports = { plugins: [ new DependencyExtractionWebpackPlugin( { requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress/webpack.config.js index 59d4c5d2ead3ab..bf6cb387dbd18f 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/wordpress/webpack.config.js @@ -7,7 +7,9 @@ module.exports = { plugins: [ new DependencyExtractionWebpackPlugin( { requestToExternalModule( request ) { - return request.startsWith( '@wordpress/' ); + return ( + request.startsWith( '@wordpress/' ) || request === 'lodash' + ); }, } ), ], From e0186815e5407417340f68661211ca849f072722 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 12 Jan 2024 10:42:06 +0100 Subject: [PATCH 2/6] Update snapshots --- .../test/__snapshots__/build.js.snap | 99 ++++++++++++++++--- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap b/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap index 84ed7660802ba2..f43c05d4de10dc 100644 --- a/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap +++ b/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap @@ -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`] = ` -" array('dependencies' => array('@wordpress/blob'), 'version' => 'b2c5cea79a32b3d91bf8', 'type' => 'module'), 'fileB.mjs' => array('dependencies' => array('@wordpress/token-list'), 'version' => '5c4197fd48811f25807f', 'type' => 'module')); +" array('dependencies' => array('@wordpress/blob', 'lodash'), 'version' => '4ab8cc4b6b7619053443', 'type' => 'module'), 'fileB.mjs' => array('dependencies' => array('@wordpress/token-list'), 'version' => '5c4197fd48811f25807f', 'type' => 'module')); " `; @@ -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", + }, ] `; @@ -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 + +ERROR in ./index.js 6:23-55 +Module not found: Error: Attempted to use WordPress script in a module: @wordpress/api-fetch +" +`; + exports[`DependencyExtractionWebpackPlugin modules Webpack \`function-output-filename\` should produce expected output: Asset file 'chunk--main--main.asset.php' should match snapshot 1`] = ` -" array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module'); " `; @@ -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`] = ` -" array('@wordpress/blob'), 'version' => '26d6da43027f3522b0ca', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => '8308f1ac78c21f09c721', 'type' => 'module'); " `; @@ -92,6 +111,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`has-extension-suffi "request": "@wordpress/blob", "userRequest": "@wordpress/blob", }, + { + "externalType": "import", + "request": "lodash", + "userRequest": "lodash", + }, ] `; @@ -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`] = ` -" array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module'); " `; @@ -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`] = ` -" array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module'); " `; @@ -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`] = ` " array('@wordpress/blob', '@wordpress/url', 'rxjs', 'rxjs/operators'), 'version' => '259fc706528651fc00c1', 'type' => 'module'); @@ -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`] = ` -" array('@wordpress/blob'), 'version' => 'a0ec8ef279476bb51e19', 'type' => 'module'); +" 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`] = ` -" array(), 'version' => '0bb8a9fae3dcfcc1ac38', 'type' => 'module'); +" array(), 'version' => 'e7402f5608a888d8fd66', 'type' => 'module'); " `; @@ -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`] = ` -" array('@wordpress/blob'), 'version' => '38479966fb62d588f05e', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => '22d18a3461df47fbaa79', 'type' => 'module'); " `; @@ -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`] = ` -" array('@wordpress/blob'), 'version' => '2925e30449840a5a80f8', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => 'e325da3aa7dbb0c0c151', 'type' => 'module'); " `; @@ -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`] = ` -" array(array('id' => '@wordpress/interactivity', 'type' => 'dynamic')), 'version' => 'f0242eb6da78af6ca4b8', 'type' => 'module'); +" 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", @@ -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`] = ` -" array('@wordpress/blob'), 'version' => '52c1849898b74d94f025', 'type' => 'module'); +" array('@wordpress/blob', 'lodash'), 'version' => 'f40de15d66b54da440d2', 'type' => 'module'); " `; @@ -275,6 +337,11 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`wordpress-require\` "request": "@wordpress/blob", "userRequest": "@wordpress/blob", }, + { + "externalType": "import", + "request": "lodash", + "userRequest": "lodash", + }, ] `; @@ -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`] = ` " array('lodash', 'wp-blob'), 'version' => 'fc2d750fc9e08c5749db'); " From e4f147e6efccb67032111c3564087e55c3bf8e78 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 12 Jan 2024 10:55:56 +0100 Subject: [PATCH 3/6] Improve requestToExternalModule shorthand handling --- .../lib/index.js | 17 ++++++++++++----- .../lib/util.js | 5 +++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/dependency-extraction-webpack-plugin/lib/index.js b/packages/dependency-extraction-webpack-plugin/lib/index.js index ea078f34a65a1c..fe9baf787e2446 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/index.js +++ b/packages/dependency-extraction-webpack-plugin/lib/index.js @@ -67,22 +67,29 @@ class DependencyExtractionWebpackPlugin { 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 ); } // Cascade to default if unhandled and enabled. - if ( ! externalRequest && this.options.useDefaults ) { + if ( + typeof externalRequest === 'undefined' && + this.options.useDefaults + ) { externalRequest = this.useModules ? defaultRequestToExternalModule( request ) : defaultRequestToExternal( request ); } - if ( this.useModules && externalRequest === true ) { - externalRequest = request; - } - if ( externalRequest instanceof Error ) { return callback( externalRequest ); } diff --git a/packages/dependency-extraction-webpack-plugin/lib/util.js b/packages/dependency-extraction-webpack-plugin/lib/util.js index 1f314ea9eb28b5..716804a3463fb4 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/util.js +++ b/packages/dependency-extraction-webpack-plugin/lib/util.js @@ -61,10 +61,11 @@ 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|boolean|Error|undefined} The resulting external definition. + * @return {string|Error|undefined} The resulting external definition. * - Return `undefined` to ignore the request (do not externalize). - * - Return `true` to externalize the request with the same Module ID * - Return `string` to map the request to an external. * - Return `Error` to emit an error. */ From 602162be069bf9c151ee8321bbd312053f47885c Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 12 Jan 2024 12:19:44 +0100 Subject: [PATCH 4/6] Throw and handle errors from requestToExternal --- .../lib/index.js | 56 ++++++++++--------- .../lib/util.js | 2 +- .../test/fixtures/error/webpack.config.js | 2 +- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/packages/dependency-extraction-webpack-plugin/lib/index.js b/packages/dependency-extraction-webpack-plugin/lib/index.js index fe9baf787e2446..344d4de9a0572e 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/index.js +++ b/packages/dependency-extraction-webpack-plugin/lib/index.js @@ -62,36 +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 ); - - // requestToExternalModule allows a boolean shorthand - if ( externalRequest === false ) { - externalRequest = undefined; - } - if ( externalRequest === true ) { - externalRequest = 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 ( externalRequest instanceof Error ) { - return callback( externalRequest ); + // 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 ) { diff --git a/packages/dependency-extraction-webpack-plugin/lib/util.js b/packages/dependency-extraction-webpack-plugin/lib/util.js index 716804a3463fb4..49b3c8e54a47c5 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/util.js +++ b/packages/dependency-extraction-webpack-plugin/lib/util.js @@ -81,7 +81,7 @@ function defaultRequestToExternalModule( request ) { const isWordPressScript = Boolean( defaultRequestToExternal( request ) ); if ( isWordPressScript ) { - return new Error( + throw new Error( `Attempted to use WordPress script in a module: ${ request }` ); } diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js index b3583f7b0a1e6f..5f2e5f899237c2 100644 --- a/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/error/webpack.config.js @@ -8,7 +8,7 @@ module.exports = { new DependencyExtractionWebpackPlugin( { // eslint-disable-next-line no-unused-vars requestToExternal( request ) { - return new Error( 'Ensure error in script build.' ); + throw new Error( 'Ensure error in script build.' ); }, } ), ], From 5860e6ac271e2a7ca43c089b04f95732dc95fa80 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 12 Jan 2024 12:37:40 +0100 Subject: [PATCH 5/6] Add "not supported yet" to error message Co-authored-by: Luis Herranz --- packages/dependency-extraction-webpack-plugin/lib/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dependency-extraction-webpack-plugin/lib/util.js b/packages/dependency-extraction-webpack-plugin/lib/util.js index 49b3c8e54a47c5..39b2f4b4bca587 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/util.js +++ b/packages/dependency-extraction-webpack-plugin/lib/util.js @@ -82,7 +82,7 @@ function defaultRequestToExternalModule( request ) { if ( isWordPressScript ) { throw new Error( - `Attempted to use WordPress script in a module: ${ request }` + `Attempted to use WordPress script in a module: ${ request }, which is not supported yet.` ); } } From 6d26ec11ff69a8689091445d399cee535599f264 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 12 Jan 2024 13:21:49 +0100 Subject: [PATCH 6/6] update snapshots --- .../test/__snapshots__/build.js.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap b/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap index f43c05d4de10dc..a595a948ba1087 100644 --- a/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap +++ b/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap @@ -72,10 +72,10 @@ 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 +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 +Module not found: Error: Attempted to use WordPress script in a module: @wordpress/api-fetch, which is not supported yet. " `;