Skip to content
Prev Previous commit
Next Next commit
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.
  • Loading branch information
michalczaplinski committed Sep 16, 2024
commit 8fd700f3ec475a0256e2d941e68e0a89491871e3
40 changes: 31 additions & 9 deletions packages/dependency-extraction-webpack-plugin/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,25 @@ class DependencyExtractionWebpackPlugin {
/**
* Can we trace a line of static dependencies from an entry to a module
*
* @param {webpack.Compilation} compilation
* @param {webpack.DependenciesBlock} block
* @param {webpack.Compilation} compilation
* @param {webpack.DependenciesBlock} block
* @param {Set<webpack.DependenciesBlock>} visited
Copy link
Contributor Author

@michalczaplinski michalczaplinski Sep 16, 2024

Choose a reason for hiding this comment

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

It might be better to only store IDs of each visited block instead of the whole block. This would use less memory.

However, I could not see if the DependenciesBlock has a stable ID of any kind: https://github.com/webpack/webpack/blob/899f06934391baede59da3dcd35b5ef51c675dbe/lib/DependenciesBlock.js#L29

Without benchmarking, it's also hard to know if it makes a difference.

Copy link
Member

Choose a reason for hiding this comment

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

I have some ideas for this, I'm working on it now.

*
* @return {boolean} True if there is a static import path to the root
*/
static hasStaticDependencyPathToRoot( compilation, block ) {
static hasStaticDependencyPathToRoot(
compilation,
block,
visited = new Set()
) {
// If we've already visited this block, we're in a cycle
if ( visited.has( block ) ) {
return false;
}

// Add the current block to the visited set
visited.add( block );

const incomingConnections = [
...compilation.moduleGraph.getIncomingConnections( block ),
].filter(
Expand All @@ -369,8 +382,9 @@ class DependencyExtractionWebpackPlugin {
);

// If we don't have non-entry, non-library incoming connections,
// we've reached a root of
// we've reached a root
if ( ! incomingConnections.length ) {
visited.delete( block );
return true;
}

Expand All @@ -389,16 +403,24 @@ class DependencyExtractionWebpackPlugin {

// All the dependencies were Async, the module was reached via a dynamic import
if ( ! staticDependentModules.length ) {
visited.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,
visited
)
);

// Remove the current block from the visited set before returning
visited.delete( block );

return result;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' => '59584257c15ebb59974d', 'type' => 'module');
"
`;

exports[`DependencyExtractionWebpackPlugin modules Webpack \`cyclic-external-deps\` should produce expected output: External modules should match snapshot 1`] = `
[
{
"externalType": "import",
"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');
"
Expand Down Expand Up @@ -394,12 +409,12 @@ exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-dependency-g
]
`;

exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-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-dynamic-dependency-graph\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
"<?php return array('dependencies' => array('wp-interactivity'), 'version' => 'ac0e2f1bcd3a6a0e7aff');
"
`;

exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-deps\` should produce expected output: External modules should match snapshot 1`] = `
exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-dynamic-dependency-graph\` should produce expected output: External modules should match snapshot 1`] = `
[
{
"externalType": "window",
Expand All @@ -412,12 +427,12 @@ exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-deps\` shoul
]
`;

exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-dynamic-dependency-graph\` should produce expected output: Asset file 'main.asset.php' should match snapshot 1`] = `
"<?php return array('dependencies' => array('wp-interactivity'), 'version' => 'ac0e2f1bcd3a6a0e7aff');
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-dynamic-dependency-graph\` should produce expected output: External modules should match snapshot 1`] = `
exports[`DependencyExtractionWebpackPlugin scripts Webpack \`cyclic-external-deps\` should produce expected output: External modules should match snapshot 1`] = `
[
{
"externalType": "window",
Expand Down