Skip to content

hoistingLimits - optimize hoisting when transitive dependency matches root one#7196

Open
Knagis wants to merge 4 commits into
yarnpkg:masterfrom
Knagis:fix-6662-optimize
Open

hoistingLimits - optimize hoisting when transitive dependency matches root one#7196
Knagis wants to merge 4 commits into
yarnpkg:masterfrom
Knagis:fix-6662-optimize

Conversation

@Knagis

@Knagis Knagis commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

This PR follows up on #7189 - this version makes sure that the transitive dependencies are still hoisted up to the hoist border.

It also adds a skipped unit test for potential future fix - when a transitive dependency matches the root one, it can be "hoisted" to root - it isn't hoisting since the dependency already exists. The PR does not include an implementation for this scenario.

Resolves #6662

@larixer

larixer commented Jun 26, 2026

Copy link
Copy Markdown
Member

@Knagis Thank you for the followup PR! Does it handle root workspace only or does it support non-root workspaces too?

@Knagis

Knagis commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

i think this should work with either, it doesn't add any special meaning to root, instead it makes all hoist border nodes unique.

@larixer

larixer commented Jun 26, 2026

Copy link
Copy Markdown
Member

I see, good idea! Thank you, looks good!

@larixer

larixer commented Jul 16, 2026

Copy link
Copy Markdown
Member

@arcanis has found an issue introduced by this PR that blocks its merging:

https://github.com/yarnpkg/berry/actions/runs/27975705591/job/82793047165?pr=7196

   ➀ YN0000: · Yarn 0.0.0
   ➀ YN0000: β”Œ Resolution step
   ➀ YN0000: β”” Completed
   ➀ YN0000: β”Œ Fetch step
   ➀ YN0000: β”” Completed
   ➀ YN0000: β”Œ Link step
   ➀ YN0001: β”‚ RangeError: Maximum call stack size exceeded

     163660 |     };
     163661 |     const seenNodes = /* @__PURE__ */ new Map([[tree, treeCopy]]);
   > 163662 |     const addNode = (node, parent, parentNode) => {
            |                     ^
     163663 |       const isHoistBorder = options2.hoistingLimits.get(parentNode.locator)?.has(node.name) || false;
     163664 |       let workNode = parent === node ? parentNode : isHoistBorder ? void 0 : seenNodes.get(node);
     163665 |       const isSeen = !!workNode;

     at addNode (../yarnpkg-cli/bundles/yarn.js:163662:21)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     at addNode (../yarnpkg-cli/bundles/yarn.js:163693:11)
     ➀ YN0000: β”” Completed
     ➀ YN0000: · Failed with errors
     ```

@larixer larixer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please fix failing integration tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug?]: nmHoistingLimits: dependencies incorrectly hoists past some dependencies with @scope names

2 participants