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 b5e33db. * Remove redundant plugin config in test * Updated the snapshot files --------- Co-authored-by: michalczaplinski <czapla@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
WordPress · Sep 19, 2024 · f0cd217 · f0cd217
1 parent cbcc28c
commit f0cd217
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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`] = `
+"<?php return array('dependencies' => 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`] = `
 "<?php return array('dependencies' => 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`] = `
+"<?php return array('dependencies' => 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`] = `
 "<?php return array('dependencies' => 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
@@ -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
@@ -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/...dependency-extraction-webpack-plugin/test/fixtures/cyclic-external-deps/webpack.config.js b/...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() ],
+};