From f0cd2174238a40bbc15812e9270bed6336fb8be8 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 19 Sep 2024 22:20:41 +0200 Subject: [PATCH] DEWP: Handle cyclical module dependencies (#65291) * Add tests for cyclical dependencies * Add the externals and make the test fail * Add cycle detection in dependency path checking Enhanced the dependency resolution logic to detect cycles in the module graph, preventing infinite loops during static dependency path checks. Introduced a Set to track visited blocks and avoid revisiting them. * Revert changes * Propose static WeakSet/WeakMap implementation. * Add CHANGELOG entry * Remove redundant plugin config in test * Revert "Remove redundant plugin config in test" This reverts commit b5e33dbc0e6db4d4aab07de4d52b42ae6b9672ee. * Remove redundant plugin config in test * Updated the snapshot files --------- Co-authored-by: michalczaplinski Co-authored-by: sirreal --- .../CHANGELOG.md | 4 ++ .../lib/index.js | 46 +++++++++++++++++-- .../test/__snapshots__/build.js.snap | 33 +++++++++++++ .../test/fixtures/cyclic-external-deps/a.js | 8 ++++ .../fixtures/cyclic-external-deps/index.js | 18 ++++++++ .../cyclic-external-deps/webpack.config.js | 8 ++++ 6 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/a.js create mode 100644 packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/index.js create mode 100644 packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/webpack.config.js diff --git a/packages/dependency-extraction-webpack-plugin/CHANGELOG.md b/packages/dependency-extraction-webpack-plugin/CHANGELOG.md index 85498d539317f..1c0aa630495a0 100644 --- a/packages/dependency-extraction-webpack-plugin/CHANGELOG.md +++ b/packages/dependency-extraction-webpack-plugin/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Bug Fixes + +- Fix a bug where cycles in dependent modules could enter infinite recursion ([#65291](https://github.com/WordPress/gutenberg/pull/65291)). + ## 6.8.0 (2024-09-19) ## 6.7.0 (2024-09-05) diff --git a/packages/dependency-extraction-webpack-plugin/lib/index.js b/packages/dependency-extraction-webpack-plugin/lib/index.js index 575882a1dfbeb..529fb339d15a1 100644 --- a/packages/dependency-extraction-webpack-plugin/lib/index.js +++ b/packages/dependency-extraction-webpack-plugin/lib/index.js @@ -369,6 +369,9 @@ class DependencyExtractionWebpackPlugin { } } + static #staticDepsCurrent = new WeakSet(); + static #staticDepsCache = new WeakMap(); + /** * Can we trace a line of static dependencies from an entry to a module * @@ -378,6 +381,20 @@ class DependencyExtractionWebpackPlugin { * @return {boolean} True if there is a static import path to the root */ static hasStaticDependencyPathToRoot( compilation, block ) { + if ( DependencyExtractionWebpackPlugin.#staticDepsCache.has( block ) ) { + return DependencyExtractionWebpackPlugin.#staticDepsCache.get( + block + ); + } + + if ( + DependencyExtractionWebpackPlugin.#staticDepsCurrent.has( block ) + ) { + return false; + } + + DependencyExtractionWebpackPlugin.#staticDepsCurrent.add( block ); + const incomingConnections = [ ...compilation.moduleGraph.getIncomingConnections( block ), ].filter( @@ -391,6 +408,13 @@ class DependencyExtractionWebpackPlugin { // If we don't have non-entry, non-library incoming connections, // we've reached a root of if ( ! incomingConnections.length ) { + DependencyExtractionWebpackPlugin.#staticDepsCache.set( + block, + true + ); + DependencyExtractionWebpackPlugin.#staticDepsCurrent.delete( + block + ); return true; } @@ -409,16 +433,28 @@ class DependencyExtractionWebpackPlugin { // All the dependencies were Async, the module was reached via a dynamic import if ( ! staticDependentModules.length ) { + DependencyExtractionWebpackPlugin.#staticDepsCache.set( + block, + false + ); + DependencyExtractionWebpackPlugin.#staticDepsCurrent.delete( + block + ); return false; } // Continue to explore any static dependencies - return staticDependentModules.some( ( parentStaticDependentModule ) => - DependencyExtractionWebpackPlugin.hasStaticDependencyPathToRoot( - compilation, - parentStaticDependentModule - ) + const result = staticDependentModules.some( + ( parentStaticDependentModule ) => + DependencyExtractionWebpackPlugin.hasStaticDependencyPathToRoot( + compilation, + parentStaticDependentModule + ) ); + + DependencyExtractionWebpackPlugin.#staticDepsCache.set( block, result ); + DependencyExtractionWebpackPlugin.#staticDepsCurrent.delete( block ); + return result; } } 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 c4b450683572e..903c9658250b1 100644 --- a/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap +++ b/packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap @@ -55,6 +55,21 @@ exports[`DependencyExtractionWebpackPlugin modules Webpack \`cyclic-dynamic-depe ] `; +exports[`DependencyExtractionWebpackPlugin modules Webpack \`cyclic-external-deps\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = ` +" array(array('id' => '@wordpress/interactivity', 'import' => 'dynamic')), 'version' => 'e1033c1bd62e8cb8d4c9', 'type' => 'module'); +" +`; + +exports[`DependencyExtractionWebpackPlugin modules Webpack \`cyclic-external-deps\` should produce expected output: External modules should match snapshot 1`] = ` +[ + { + "externalType": "module", + "request": "@wordpress/interactivity", + "userRequest": "@wordpress/interactivity", + }, +] +`; + exports[`DependencyExtractionWebpackPlugin modules Webpack \`dynamic-import\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = ` " array(array('id' => '@wordpress/blob', 'import' => 'dynamic')), 'version' => '4f59b7847b70a07b2710', 'type' => 'module'); " @@ -419,6 +434,24 @@ exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-dynamic-depe ] `; +exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-external-deps\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = ` +" array('wp-interactivity'), 'version' => '455f3cab924853d41b8b'); +" +`; + +exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-external-deps\` should produce expected output: External modules should match snapshot 1`] = ` +[ + { + "externalType": "window", + "request": [ + "wp", + "interactivity", + ], + "userRequest": "@wordpress/interactivity", + }, +] +`; + exports[`DependencyExtractionWebpackPlugin scripts Webpack \`dynamic-import\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = ` " array('wp-blob'), 'version' => 'c0e8a6f22065ea096606'); " diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/a.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/a.js new file mode 100644 index 0000000000000..1f0edffe27efd --- /dev/null +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/a.js @@ -0,0 +1,8 @@ +/** + * Internal dependencies + */ +import { someFunction } from '.'; + +someFunction(); + +export const a = 'test'; diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/index.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/index.js new file mode 100644 index 0000000000000..01d7eff466bfb --- /dev/null +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/index.js @@ -0,0 +1,18 @@ +/** + * Internal dependencies + */ +import { a } from './a'; + +/** + * WordPress dependencies + */ +import { store } from '@wordpress/interactivity'; + +export const someFunction = () => { + store( 'test', { + state: { + a, + }, + } ); + return a; +}; diff --git a/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/webpack.config.js b/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/webpack.config.js new file mode 100644 index 0000000000000..bfffff3ae7831 --- /dev/null +++ b/packages/dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/webpack.config.js @@ -0,0 +1,8 @@ +/** + * Internal dependencies + */ +const DependencyExtractionWebpackPlugin = require( '../../..' ); + +module.exports = { + plugins: [ new DependencyExtractionWebpackPlugin() ], +};