fix(global virtual store): bridge the resolution paths a store slot lost

e2e/harmony/global-virtual-store.e2e.ts

@@ -56,6 +56,28 @@ describe('installing with the global virtual store', function () {
       ).to.be.a.path();
     });
   });
+  describe('building an aspect', () => {
+    let output: string;
+    before(() => {
+      helper.scopeHelper.reInitWorkspace();
+      helper.extensions.workspaceJsonc.addKeyValToDependencyResolver('enableGlobalVirtualStore', true);
+      helper.workspaceJsonc.disablePreview();
+      helper.fixtures.populateExtensions(1);
+      helper.extensions.addExtensionToVariant('extensions', 'teambit.harmony/aspect');
+      helper.command.install();
+      // throws on a failed build pipeline, which is the assertion: the TSCompiler task is what
+      // breaks when the types below cannot be reached
+      output = helper.command.tagAllComponents();
+    });
+    // the compiled program reaches `.d.ts` files that sit in store slots, and those reference
+    // types they never declare - `@types/*` for a package that ships none, the core aspects. From
+    // a store slot none of it resolves by walking up, and the types quietly degrade into errors
+    // that name a prop rather than the cause.
+    it('should type-check the aspect against the types a store slot cannot reach by walking up', () => {
+      expect(output).to.not.have.string('error TS');
+      expect(helper.command.listParsed()).to.have.lengthOf(1);
+    });
+  });
   describe('patched dependencies', () => {
     before(() => {
       helper.scopeHelper.reInitWorkspace();

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts

@@ -1,6 +1,15 @@
 import { expect } from 'chai';
+import fs from 'fs-extra';
+import Module from 'module';
+import os from 'os';
 import path from 'path';
-import { isPathInsideOrEqual, parseRecordedVirtualStoreDir } from './hoisted-resolution-bridge';
+import {
+  ensureHoistedDependencyResolution,
+  hoistedResolutionDirs,
+  isPathInsideOrEqual,
+  isSamePath,
+  parseRecordedVirtualStoreDir,
+} from './hoisted-resolution-bridge';
 
 describe('isPathInsideOrEqual()', () => {
   const base = path.resolve('/base');
@@ -36,3 +45,171 @@ describe('parseRecordedVirtualStoreDir()', () => {
     expect(parseRecordedVirtualStoreDir(JSON.stringify({ layoutVersion: 5 }))).to.eq(undefined);
   });
 });
+
+describe('hoistedResolutionDirs()', () => {
+  let root: string;
+  const hoisted = () => path.join(root, 'node_modules', '.pnpm', 'node_modules');
+  const rootModules = () => path.join(root, 'node_modules');
+
+  beforeEach(() => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'hoisted-resolution-dirs-'));
+  });
+  afterEach(() => fs.removeSync(root));
+
+  it('should return both directories in the order the walk reached them', () => {
+    fs.ensureDirSync(hoisted());
+    expect(hoistedResolutionDirs(root)).to.deep.eq([hoisted(), rootModules()]);
+  });
+  it('should keep the root node_modules when nothing was hoisted', () => {
+    fs.ensureDirSync(rootModules());
+    expect(hoistedResolutionDirs(root)).to.deep.eq([rootModules()]);
+  });
+  it('should return nothing for a root that was never installed', () => {
+    expect(hoistedResolutionDirs(root)).to.deep.eq([]);
+  });
+});
+
+describe('ensureHoistedDependencyResolution()', () => {
+  let root: string;
+  let nodePath: string | undefined;
+  let nodeOptions: string | undefined;
+  let register: unknown;
+  // the two process-global side effects of the function under test, neither of them scoped to a
+  // test: `_initPaths()` rederives Module.globalPaths from NODE_PATH, and `module.register()`
+  // installs an ESM loader that cannot be removed for the life of the process
+  const nodeModule = Module as unknown as { register?: unknown; _initPaths(): void };
+  const hoisted = () => path.join(root, 'node_modules', '.pnpm', 'node_modules');
+  const rootModules = () => path.join(root, 'node_modules');
+  const entries = () => (process.env.NODE_PATH ?? '').split(path.delimiter).filter(Boolean);
+
+  beforeEach(() => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'ensure-hoisted-resolution-'));
+    fs.ensureDirSync(hoisted());
+    nodePath = process.env.NODE_PATH;
+    nodeOptions = process.env.NODE_OPTIONS;
+    // these cases are about NODE_PATH order; taking `register` away keeps the ESM half - the
+    // irreversible half - out of the test process, through the same guard that carries older
+    // runtimes
+    register = nodeModule.register;
+    nodeModule.register = undefined;
+  });
+  afterEach(() => {
+    if (nodePath === undefined) delete process.env.NODE_PATH;
+    else process.env.NODE_PATH = nodePath;
+    if (nodeOptions === undefined) delete process.env.NODE_OPTIONS;
+    else process.env.NODE_OPTIONS = nodeOptions;
+    nodeModule.register = register;
+    // restoring the variable is not enough: the resolver reads the paths derived from it, which
+    // would otherwise still point into the directory removed on the next line
+    nodeModule._initPaths();
+    fs.removeSync(root);
+  });
+
+  it('should put both directories in walk order', () => {
+    delete process.env.NODE_PATH;
+    ensureHoistedDependencyResolution(root);
+    expect(entries()).to.deep.eq([hoisted(), rootModules()]);
+  });
+
+  it('should reorder entries a previous bridge left in the wrong order', () => {
+    // a bit that bridged the hoisted directory alone leaves it in NODE_PATH for its children;
+    // adding the root's node_modules in front of it there would invert the walk
+    process.env.NODE_PATH = hoisted();
+    ensureHoistedDependencyResolution(root);
+    expect(entries()).to.deep.eq([hoisted(), rootModules()]);
+  });
+
+  it('should keep entries it does not own, behind its own', () => {
+    const foreign = path.join(root, 'somewhere-else');
+    process.env.NODE_PATH = [rootModules(), foreign].join(path.delimiter);
+    ensureHoistedDependencyResolution(root);
+    expect(entries()).to.deep.eq([hoisted(), rootModules(), foreign]);
+  });
+
+  it('should replace an entry that names an owned directory in another spelling', () => {
+    process.env.NODE_PATH = [`${rootModules()}${path.sep}`, `${hoisted()}${path.sep}.`].join(path.delimiter);
+    ensureHoistedDependencyResolution(root);
+    expect(entries()).to.deep.eq([hoisted(), rootModules()]);
+  });
+
+  it('should leave NODE_PATH untouched when it already reads correctly', () => {
+    process.env.NODE_PATH = [hoisted(), rootModules()].join(path.delimiter);
+    const before = process.env.NODE_PATH;
+    ensureHoistedDependencyResolution(root);
+    expect(process.env.NODE_PATH).to.eq(before);
+  });
+
+  it('should do nothing for a root that was never installed', () => {
+    const bare = fs.mkdtempSync(path.join(os.tmpdir(), 'ensure-hoisted-resolution-bare-'));
+    delete process.env.NODE_PATH;
+    try {
+      ensureHoistedDependencyResolution(bare);
+      expect(process.env.NODE_PATH).to.eq(undefined);
+    } finally {
+      fs.removeSync(bare);
+    }
+  });
+});
+
+describe('ensureHoistedDependencyResolution() esm registration', () => {
+  let first: string;
+  let second: string;
+  let nodePath: string | undefined;
+  let nodeOptions: string | undefined;
+  let register: unknown;
+  const nodeModule = Module as unknown as { register?: unknown; _initPaths(): void };
+  const flag = () => (process.env.NODE_OPTIONS ?? '').match(/--import=\S+/)?.[0];
+
+  beforeEach(() => {
+    first = fs.mkdtempSync(path.join(os.tmpdir(), 'esm-registration-first-'));
+    second = fs.mkdtempSync(path.join(os.tmpdir(), 'esm-registration-second-'));
+    [first, second].forEach((root) => fs.ensureDirSync(path.join(root, 'node_modules', '.pnpm', 'node_modules')));
+    nodePath = process.env.NODE_PATH;
+    nodeOptions = process.env.NODE_OPTIONS;
+    delete process.env.NODE_PATH;
+    delete process.env.NODE_OPTIONS;
+    register = nodeModule.register;
+    // a no-op keeps the body running - the flag is what these cases are about - without leaving a
+    // loader registered on the process
+    nodeModule.register = () => {};
+  });
+  afterEach(() => {
+    if (nodePath === undefined) delete process.env.NODE_PATH;
+    else process.env.NODE_PATH = nodePath;
+    if (nodeOptions === undefined) delete process.env.NODE_OPTIONS;
+    else process.env.NODE_OPTIONS = nodeOptions;
+    nodeModule.register = register;
+    nodeModule._initPaths();
+    [first, second].forEach((root) => fs.removeSync(root));
+  });
+
+  it('should hand children a flag carrying the order NODE_PATH now reads', () => {
+    ensureHoistedDependencyResolution(first);
+    ensureHoistedDependencyResolution(second);
+    const beforeReorder = flag();
+    // bridging the first root again moves its directories back to the front, so the list the
+    // loader was registered with no longer matches the one CommonJS resolves through
+    ensureHoistedDependencyResolution(first);
+    expect(flag()).to.not.eq(beforeReorder);
+  });
+
+  it('should leave the flag alone when nothing about the list changed', () => {
+    ensureHoistedDependencyResolution(first);
+    const unchanged = flag();
+    ensureHoistedDependencyResolution(first);
+    expect(flag()).to.eq(unchanged);
+  });
+});
+
+describe('isSamePath()', () => {
+  const dir = path.resolve('/base', 'node_modules');
+  it('should ignore a trailing separator', () => {
+    expect(isSamePath(`${dir}${path.sep}`, dir)).to.eq(true);
+  });
+  it('should ignore a redundant current-directory segment', () => {
+    expect(isSamePath(path.join(dir, '.'), dir)).to.eq(true);
+  });
+  it('should separate genuinely different directories', () => {
+    expect(isSamePath(path.join(dir, 'nested'), dir)).to.eq(false);
+  });
+});