From a22486f345c1f1fd94fd3a60ea4490c1f1bbdb0f Mon Sep 17 00:00:00 2001 From: Victor Vlasenko Date: Thu, 12 Sep 2024 15:05:15 +0300 Subject: [PATCH] fix(nm): Stop hoisting rounds only when nothing were hoisted (#6495) ## What's the problem this PR addresses? Fixes #6493 Fixes #6494 ## How did you fix it? Issue: #6493. Sometimes hoisting algorithm was stopped too early, because it wasn't able to determine correctly stop condition. I made it safer but possibly one round slower by stopping only when nothing was hoisted during the last round. Issue: #6494. Hoisting avoided hoisting to non-root workspace previously, I have removed this limitation, because it made inner workspaces meaningless. ## Checklist - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). - [x] I have set the packages that need to be released for my changes to be effective. - [x] I will check that all automated PR checks pass before the PR gets reviewed. --- .yarn/versions/40a42318.yml | 25 ++++++++++++++++ CHANGELOG.md | 2 ++ packages/yarnpkg-nm/sources/hoist.ts | 21 ++----------- packages/yarnpkg-nm/tests/hoist.test.ts | 40 +++++++++++++++++++++---- 4 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 .yarn/versions/40a42318.yml diff --git a/.yarn/versions/40a42318.yml b/.yarn/versions/40a42318.yml new file mode 100644 index 000000000000..357b83b3d975 --- /dev/null +++ b/.yarn/versions/40a42318.yml @@ -0,0 +1,25 @@ +releases: + "@yarnpkg/cli": patch + "@yarnpkg/nm": patch + "@yarnpkg/plugin-nm": patch + "@yarnpkg/pnpify": patch + +declined: + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-constraints" + - "@yarnpkg/plugin-dlx" + - "@yarnpkg/plugin-essentials" + - "@yarnpkg/plugin-init" + - "@yarnpkg/plugin-interactive-tools" + - "@yarnpkg/plugin-npm-cli" + - "@yarnpkg/plugin-pack" + - "@yarnpkg/plugin-patch" + - "@yarnpkg/plugin-pnp" + - "@yarnpkg/plugin-pnpm" + - "@yarnpkg/plugin-stage" + - "@yarnpkg/plugin-typescript" + - "@yarnpkg/plugin-version" + - "@yarnpkg/plugin-workspace-tools" + - "@yarnpkg/builder" + - "@yarnpkg/core" + - "@yarnpkg/doctor" diff --git a/CHANGELOG.md b/CHANGELOG.md index a045c4d73eac..03f75364ca89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ Features in `master` can be tried out by running `yarn set version from sources` - Fixes `preferInteractive` forcing interactive mode in non-TTY environments. - `node-modules` linker now honors user-defined symlinks for `/node_modules` directories +- `node-modules` linker supports hoisting into inner workspaces that are parents of other workspaces +- `node-modules` linker attemps to hoist tree more exhaustivel until nothing can be hoisted ## 4.1.0 diff --git a/packages/yarnpkg-nm/sources/hoist.ts b/packages/yarnpkg-nm/sources/hoist.ts index 4a0f345a1e66..014ef1dcf924 100644 --- a/packages/yarnpkg-nm/sources/hoist.ts +++ b/packages/yarnpkg-nm/sources/hoist.ts @@ -132,7 +132,8 @@ export const hoist = (tree: HoisterTree, opts: HoistOptions = {}): HoisterResult let anotherRoundNeeded = false; let round = 0; do { - anotherRoundNeeded = hoistTo(treeCopy, [treeCopy], new Set([treeCopy.locator]), new Map(), options).anotherRoundNeeded; + const result = hoistTo(treeCopy, [treeCopy], new Set([treeCopy.locator]), new Map(), options); + anotherRoundNeeded = result.anotherRoundNeeded || result.isGraphChanged; options.fastLookupPossible = false; round++; } while (anotherRoundNeeded); @@ -480,24 +481,6 @@ const getNodeHoistInfo = (rootNode: HoisterWorkTree, rootNodePathLocators: Set { expect(getTreeHeight(hoist(toTree(tree), {check: true}))).toEqual(2); }); - it(`should avoid hoisting direct workspace dependencies into non-root workspace`, () => { + it(`should hoist direct workspace dependencies into non-root workspace`, () => { // . -> W1(w) -> W2(w) -> W3(w)-> A@X // -> A@Y // -> W3 // -> A@Z - // The A@X must not be hoisted into W2(w) - // otherwise accessing A via . -> W3 with --preserve-symlinks will result in A@Z, - // but accessing it via W3(w) will result in A@Y + // The A@X must be hoisted into W2(w) + // Accessing A via . -> W3 with --preserve-symlinks will result in A@Z, + // but accessing it via W3(w) will result in A@Y, however if we don't do it, + // inner workspaces will have multiple unexpected copies of dependencies const tree = { '.': {dependencies: [`W1(w)`, `W3`, `A@Z`], dependencyKind: HoisterDependencyKind.WORKSPACE}, 'W1(w)': {dependencies: [`W2(w)`, `A@Y`], dependencyKind: HoisterDependencyKind.WORKSPACE}, @@ -549,7 +550,36 @@ describe(`hoist`, () => { 'W3(w)': {dependencies: [`A@X`], dependencyKind: HoisterDependencyKind.WORKSPACE}, }; - expect(getTreeHeight(hoist(toTree(tree), {check: true}))).toEqual(5); + expect(getTreeHeight(hoist(toTree(tree), {check: true}))).toEqual(4); + }); + + it(`should hoist dependencies to the top from workspaces that have no hoist borders given there is workspace with hoist borders`, () => { + // . -> W1(w)| -> A@X --> B + // -> B@X + // -> W2(w) -> A@Y --> B + // -> B@Y + // should be hoisted to: + // . -> W1(w)| -> A@X -->B + // -> B@X + // -> W2(w) + // -> A@Y --> B + // -> B@Y + + const tree = { + '.': {dependencies: [`W1(w)`, `W2(w)`], dependencyKind: HoisterDependencyKind.WORKSPACE}, + 'W1(w)': {dependencies: [`A@X`, `B@X`], dependencyKind: HoisterDependencyKind.WORKSPACE}, + 'A@X': {dependencies: [`B@X`], peerNames: [`B`]}, + 'A@Y': {dependencies: [`B@Y`], peerNames: [`B`]}, + 'W2(w)': {dependencies: [`A@Y`, `B@Y`], dependencyKind: HoisterDependencyKind.WORKSPACE}, + }; + + const hoistingLimits = new Map([ + [`.@`, new Set([`W1(w)`])], + ]); + + const hoistedTree = hoist(toTree(tree), {check: true, hoistingLimits}); + const W2 = Array.from(Array.from(hoistedTree.dependencies).filter(x => x.name === `W2(w)`)[0].dependencies); + expect(W2).toEqual([]); }); it(`should hoist aliased packages`, () => {