Change data-structures used by hoisting algorithm to better fit its p…

…urpose (yarnpkg#745) * Speedup hoisting by introducing micro-optimizations * Add node name itself to the list of parent node names * Commit current unfinished work * Simplify hoister * Do not activate compat plugin for node-modules linker * Add hoisting tracing for troubleshooting purposes * Avoid using mkdir in favor of mkdirp, because we have contention on dir creation. * Add multi-pass hoisting * Implements seemingly fast and correct shallow-first hoisting * Simplify things a bit * Turn off self-check by default * Run version check * Cleanup * Add failing test for node_modules hoisting with workspace conflict * Hoist pnp roots independently * Adds workspace roots support with passthrough hoisting * Fix $wsroot$ packages handling * Fix topLevel package node creation * Update packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts Co-Authored-By: Maël Nison <[email protected]> * Update packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts Co-Authored-By: Maël Nison <[email protected]> * Use early return in n_m tree building function * deps -> dependencies * Makes shrinkTree more efficient by utilizing circular refs * deps -> dependencies * Document the hoisting algorithm used Co-authored-by: Nicolò Ribaudo <[email protected]> Co-authored-by: Maël Nison <[email protected]>
bertho-zero · Feb 11, 2020 · bceb690 · bceb690
1 parent ca9d23f
commit bceb690
Show file tree

Hide file tree

Showing 9 changed files with 724 additions and 590 deletions.
diff --git a/.yarn/versions/e8b92e3f.yml b/.yarn/versions/e8b92e3f.yml
@@ -0,0 +1,25 @@
+releases:
+  "@yarnpkg/cli": prerelease
+  "@yarnpkg/plugin-compat": prerelease
+  "@yarnpkg/plugin-node-modules": prerelease
+  "@yarnpkg/plugin-pnp": prerelease
+  "@yarnpkg/pnp": prerelease
+  "@yarnpkg/pnpify": prerelease
+
+declined:
+  - "@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-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - vscode-zipfs
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts
@@ -1,7 +1,7 @@
 import {npath} from '@yarnpkg/fslib';
 import {fs}    from 'pkg-tests-core';
 
-const {writeFile} = fs;
+const {writeFile, writeJson} = fs;
 
 describe('Node_Modules', () => {
   it('should install one dependency',
@@ -22,4 +22,50 @@ describe('Node_Modules', () => {
       },
     )
   );
+
+  test(
+    `workspace packages shouldn't be hoisted if they conflict with root dependencies`,
+    makeTemporaryEnv(
+      {
+        private: true,
+        workspaces: [`packages/*`],
+        dependencies: {
+          [`no-deps`]: `*`,
+        },
+      },
+      async ({path, run, source}) => {
+        await writeFile(npath.toPortablePath(`${path}/.yarnrc.yml`), `
+          enableTransparentWorkspaces: false
+          nodeLinker: "node-modules"
+        `);
+
+        await writeFile(
+          npath.toPortablePath(`${path}/index.js`),
+          `
+            module.exports = require('no-deps/package.json');
+          `,
+        );
+
+        await writeJson(npath.toPortablePath(`${path}/packages/workspace/package.json`), {
+          name: `workspace`,
+          version: `1.0.0`,
+          dependencies: {
+            [`no-deps`]: `workspace:*`,
+          },
+        });
+
+        await writeJson(npath.toPortablePath(`${path}/packages/no-deps/package.json`), {
+          name: `no-deps`,
+          version: `1.0.0-local`,
+        });
+
+        await run(`install`);
+
+        await expect(source(`require('.')`)).resolves.toMatchObject({
+          name: `no-deps`,
+          version: `2.0.0`,
+        });
+      },
+    ),
+  );
 });
diff --git a/packages/plugin-compat/sources/index.ts b/packages/plugin-compat/sources/index.ts
@@ -15,12 +15,16 @@ const PATCHES = new Map([
 const plugin: Plugin<CoreHooks & PatchHooks> = {
   hooks: {
     registerPackageExtensions: async (configuration, registerPackageExtension) => {
+      if (configuration.get('nodeLinker') === 'node-modules')
+        return;
       for (const [descriptorStr, extensionData] of packageExtensions) {
         registerPackageExtension(structUtils.parseDescriptor(descriptorStr, true), extensionData);
       }
     },
 
     getBuiltinPatch: async (project, name) => {
+      if (project.configuration.get('nodeLinker') === 'node-modules')
+        return;
       const TAG = `compat/`;
       if (!name.startsWith(TAG))
         return;
@@ -32,6 +36,8 @@ const plugin: Plugin<CoreHooks & PatchHooks> = {
     },
 
     reduceDependency: async (dependency, project, locator, initialDescriptor) => {
+      if (project.configuration.get('nodeLinker') === 'node-modules')
+        return dependency;
       const patch = PATCHES.get(dependency.identHash);
       if (typeof patch === `undefined`)
         return dependency;

diff --git a/packages/plugin-node-modules/sources/NodeModulesLinker.ts b/packages/plugin-node-modules/sources/NodeModulesLinker.ts
@@ -169,9 +169,12 @@ async function writeInstallState(project: Project, locatorMap: NodeModulesLocato
   locatorState += `__metadata:\n`;
   locatorState += `  version: 1\n`;
 
-  for (const [locatorStr, installRecord] of locatorMap.entries()) {
+  const locators = Array.from(locatorMap.keys()).sort();
+
+  for (const locator of locators) {
+    const installRecord = locatorMap.get(locator)!;
     locatorState += `\n`;
-    locatorState += `${JSON.stringify(locatorStr)}:\n`;
+    locatorState += `${JSON.stringify(locator)}:\n`;
     locatorState += `  locations:\n`;
 
     for (const location of installRecord.locations) {
@@ -452,7 +455,7 @@ async function persistNodeModules(preinstallState: NodeModulesLocatorMap | null,
 
         if (entry.name !== NODE_MODULES || !options || !options.keepSrcNodeModules) {
           if (entry.isDirectory()) {
-            await xfs.mkdirPromise(dst);
+            await xfs.mkdirpPromise(dst);
             await cloneModule(src, dst, {keepSrcNodeModules: false, keepDstNodeModules: false, innerLoop: true});
           } else {
             await xfs.copyFilePromise(src, dst, fs.constants.COPYFILE_FICLONE);

diff --git a/packages/plugin-pnp/sources/AbstractPnpInstaller.ts b/packages/plugin-pnp/sources/AbstractPnpInstaller.ts
@@ -74,7 +74,7 @@ export abstract class AbstractPnpInstaller implements Installer {
 
     for (const descriptor of pkg.peerDependencies.values()) {
       packageDependencies.set(structUtils.requirableIdent(descriptor), null);
-      packagePeers.add(descriptor.name);
+      packagePeers.add(structUtils.stringifyIdent(descriptor));
     }
 
     miscUtils.getMapWithDefault(this.packageRegistry, key1).set(key2, {