From ec24fde28754909e792962567b3e2bedbd7c4211 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 11:23:55 +0200 Subject: [PATCH 1/8] fix(global virtual store): bridge the resolution paths a store slot lost Under the global virtual store a package's real directory sits outside the project, so the ancestor walk that used to satisfy its undeclared requires reaches nothing. The bridge restored one of the two directories that walk passed through, and only for require - not for tsc. Both directories, not just the hoisted one. pnpm hoists non-direct dependencies into node_modules/.pnpm/node_modules; a direct dependency of the root is reachable through the root's own node_modules and nowhere else, which is exactly the half that was missing. TypeScript reads neither NODE_PATH nor the ESM loader, so a .d.ts in a store slot resolves `react` to react/index.js, gets no typings, and every type derived from it degrades into errors that name a prop rather than the cause. Its only lever is `paths`, a redirect where NODE_PATH is a fallback, so the mapping is kept to what the walk actually used to find: a @types/x mapping only when x itself ships no typings - otherwise packages carrying their own modern types get dragged back to stale ones - and @teambit from the root alone, since core aspects have to be the single copy from the running installation and the hoisted directory holds older transitive ones. Both roots are gated on the layout their own last install recorded, so a project-local installation is untouched. Co-Authored-By: Claude Opus 5 (1M context) --- e2e/harmony/global-virtual-store.e2e.ts | 22 ++++ .../hoisted-resolution-bridge.spec.ts | 27 ++++- .../hoisted-resolution-bridge.ts | 68 ++++++++---- .../dependencies/dependency-resolver/index.ts | 8 +- .../global-virtual-store-type-paths.spec.ts | 91 +++++++++++++++ .../global-virtual-store-type-paths.ts | 104 ++++++++++++++++++ .../typescript/typescript.main.runtime.ts | 36 +++++- 7 files changed, 331 insertions(+), 25 deletions(-) create mode 100644 scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts create mode 100644 scopes/typescript/typescript/global-virtual-store-type-paths.ts diff --git a/e2e/harmony/global-virtual-store.e2e.ts b/e2e/harmony/global-virtual-store.e2e.ts index b5ed16f38a64..22d89068d370 100644 --- a/e2e/harmony/global-virtual-store.e2e.ts +++ b/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(); diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts index 7223d42b152e..0024bf0276c6 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts @@ -1,6 +1,8 @@ import { expect } from 'chai'; +import fs from 'fs-extra'; +import os from 'os'; import path from 'path'; -import { isPathInsideOrEqual, parseRecordedVirtualStoreDir } from './hoisted-resolution-bridge'; +import { hoistedResolutionDirs, isPathInsideOrEqual, parseRecordedVirtualStoreDir } from './hoisted-resolution-bridge'; describe('isPathInsideOrEqual()', () => { const base = path.resolve('/base'); @@ -36,3 +38,26 @@ 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([]); + }); +}); diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts index 640fdca6e417..1e27e5e8b593 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts @@ -10,15 +10,22 @@ * This covers phantom dependencies generally, not one class of them. Bit's core aspects are the * case that motivated it - `@teambit/*` is required by every published env and aspect without * being declared, because it has to be the single copy from the running installation - but any - * under-declared package in the graph resolves the same way, which is why the whole hoisted - * directory goes on the path rather than a hand-picked list. + * under-declared package in the graph resolves the same way, which is why whole directories go on + * the path rather than a hand-picked list. + * + * Both directories the walk reached are restored ({@link hoistedResolutionDirs}), because they + * hold disjoint sets: pnpm hoists only *non-direct* dependencies, so a direct dependency of the + * root - the workspace's own `mocha`, `react`, `@types/react` - is reachable through the root's + * `node_modules` and nowhere else. * * pnpm's answer for this layout is `NODE_PATH` pointing at the hoisted directory, which stays * project-local under the global virtual store. pnpm sets it in the command shims it writes, but * bit runs from bvm rather than through a shim and loads aspects in its own process, so it has to * do this itself. `NODE_PATH` only covers CommonJS; an ESM loader (adapted from pnpm's * `@pnpm/plugin-esm-node-path`, MIT) handles the ESM side, registered both in-process via - * `module.register` and for child processes via a `NODE_OPTIONS --import` flag. + * `module.register` and for child processes via a `NODE_OPTIONS --import` flag. TypeScript reads + * neither, so the type-resolution half lives in `@teambit/typescript` and builds its `paths` from + * the same directories. * * Two call sites, and both matter: * - `bootstrap()` in `@teambit/bit`, before any aspect loads, gated on the layout the last @@ -29,7 +36,7 @@ * old layout, yet the same process goes on to reload envs and compile from store slots. The * installer re-applies the bridge the moment the target layout is known. * - * Everything here is idempotent, so calling it again is always safe: the `NODE_PATH` entry is + * Everything here is idempotent, so calling it again is always safe: each `NODE_PATH` entry is * added once, the ESM loader re-registers only when the entry set grows (the previous, subset * loader keeps chaining harmlessly), and the process's own `NODE_OPTIONS` flag is replaced * rather than stacked. @@ -69,9 +76,10 @@ export async function resolve (specifier, context, defaultResolve) { } // createRequire anchored inside the entry searches the entry itself: Node skips appending // /node_modules to a directory already named node_modules, and the parent step re-yields it - // (.pnpm -> .pnpm/node_modules). Exact for the hoisted-dir entries this loader exists for; - // an entry not named node_modules would be searched one level deeper than CommonJS NODE_PATH - // does - those entries are already covered by the CommonJS side and are not ours to fix. + // (.pnpm -> .pnpm/node_modules). Exact for the entries this loader exists for, both of which + // are named node_modules; an entry not named node_modules would be searched one level deeper + // than CommonJS NODE_PATH does - those entries are already covered by the CommonJS side and + // are not ours to fix. for (const basePath of extraNodePaths) { try { const require = createRequire(pathToFileURL(basePath + '/').href) @@ -167,7 +175,7 @@ export function isGlobalVirtualStoreLayout(workspaceRoot: string): boolean { * setups that invoke bit through a resolved path (and for the copy-shaped installations where it * works fine). `undefined` when neither leads to an installation (e.g. running from source). */ -function selfInstallationRoot(): string | undefined { +export function selfInstallationRoot(): string | undefined { const candidates = [process.argv[1], __dirname].filter(Boolean) as string[]; for (const candidate of candidates) { let dir = path.dirname(path.resolve(candidate)); @@ -182,10 +190,10 @@ function selfInstallationRoot(): string | undefined { } /** - * Bridge the *running installation's* own hoisted directory when that installation is itself - * linked to the global virtual store - its packages then live in store slots and their phantom - * requires need the installation's private hoist exactly like a workspace's do. Judged by the - * same `.modules.yaml` record, read at the installation root. A conventional project-local + * Bridge the *running installation's* own directories when that installation is itself linked to + * the global virtual store - its packages then live in store slots and their phantom requires + * need the installation's private hoist exactly like a workspace's do. Judged by the same + * `.modules.yaml` record, read at the installation root. A conventional project-local * installation (bvm's default) reads project-local and stays unbridged, so nothing changes for * it or for the processes it spawns. */ @@ -197,17 +205,37 @@ export function ensureSelfInstallationBridge(): void { } /** - * Put the given root's hoisted directory on the resolution path of this process and its children. - * Idempotent; a no-op when the hoisted directory doesn't exist. Callers decide *whether* the + * The directories a package resolved from `` used to reach by walking up, in the order the + * walk reached them: the hoisted `node_modules/.pnpm/node_modules` first, then the root's own + * `node_modules`. Both are lost under the global virtual store, and they hold disjoint sets - + * pnpm hoists only *non-direct* dependencies, so a direct dependency of the root exists solely in + * the second. Missing directories are dropped; a root with neither yields an empty list. + * + * Shared with the type-resolution side of the bridge, which has to reach the same two directories + * through the mechanism TypeScript offers instead of `NODE_PATH`. + */ +export function hoistedResolutionDirs(root: string): string[] { + return [path.join(root, 'node_modules', '.pnpm', 'node_modules'), path.join(root, 'node_modules')].filter((dir) => + fs.existsSync(dir) + ); +} + +/** + * Put the given root's {@link hoistedResolutionDirs} on the resolution path of this process and + * its children. Idempotent; a no-op when neither directory exists. Callers decide *whether* the * root needs the bridge ({@link isGlobalVirtualStoreLayout} for the pre-aspect gate, the * dependency-resolver's own config after an install). */ export function ensureHoistedDependencyResolution(workspaceRoot: string): void { - const hoistedDir = path.join(workspaceRoot, 'node_modules', '.pnpm', 'node_modules'); - if (!fs.existsSync(hoistedDir)) return; + const dirs = hoistedResolutionDirs(workspaceRoot); + if (!dirs.length) return; const existing = process.env.NODE_PATH; - if (!existing?.split(path.delimiter).includes(hoistedDir)) { - process.env.NODE_PATH = existing ? `${hoistedDir}${path.delimiter}${existing}` : hoistedDir; + const known = new Set(existing?.split(path.delimiter).filter(Boolean) ?? []); + const added = dirs.filter((dir) => !known.has(dir)); + if (added.length) { + // prepended in walk order, so the hoisted directory keeps winning over the root's own + // node_modules exactly as the walk out of `.pnpm` used to reach it first + process.env.NODE_PATH = [...added, ...(existing ? [existing] : [])].join(path.delimiter); // `NODE_PATH` is read once when the module system initializes, so a later assignment only // takes effect after re-deriving the global paths. (require('module') as { _initPaths(): void })._initPaths(); @@ -258,9 +286,7 @@ function registerEsmNodePathLoader(): void { if (lastImportFlag && process.env.NODE_OPTIONS?.includes(lastImportFlag)) { process.env.NODE_OPTIONS = process.env.NODE_OPTIONS.replace(lastImportFlag, importFlag); } else if (!process.env.NODE_OPTIONS?.includes(importFlag)) { - process.env.NODE_OPTIONS = process.env.NODE_OPTIONS - ? `${process.env.NODE_OPTIONS} ${importFlag}` - : importFlag; + process.env.NODE_OPTIONS = process.env.NODE_OPTIONS ? `${process.env.NODE_OPTIONS} ${importFlag}` : importFlag; } lastImportFlag = importFlag; } diff --git a/scopes/dependencies/dependency-resolver/index.ts b/scopes/dependencies/dependency-resolver/index.ts index aadecd66a668..b8b6db867f33 100644 --- a/scopes/dependencies/dependency-resolver/index.ts +++ b/scopes/dependencies/dependency-resolver/index.ts @@ -80,5 +80,11 @@ export { extendWithComponentsFromDir } from './extend-with-components-from-dir'; export { isRange } from './manifest/deduping/hoist-dependencies'; export type { DependencyEnv } from './dependency-env'; export { DetectorHook, DependencyDetector, FileContext } from './detector-hook'; -export { ensureHoistedDependencyResolution, ensureSelfInstallationBridge, isGlobalVirtualStoreLayout } from './hoisted-resolution-bridge'; +export { + ensureHoistedDependencyResolution, + ensureSelfInstallationBridge, + hoistedResolutionDirs, + isGlobalVirtualStoreLayout, + selfInstallationRoot, +} from './hoisted-resolution-bridge'; export { DependencyResolverAspect as default, DependencyResolverAspect }; diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts new file mode 100644 index 000000000000..997f688e7ccf --- /dev/null +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts @@ -0,0 +1,91 @@ +import { expect } from 'chai'; +import fs from 'fs-extra'; +import os from 'os'; +import path from 'path'; +import { globalVirtualStoreTypePaths, mergeTypePaths, typesDirToSpecifier } from './global-virtual-store-type-paths'; + +describe('typesDirToSpecifier()', () => { + it('should leave an unscoped name alone', () => { + expect(typesDirToSpecifier('react')).to.eq('react'); + }); + it('should unmangle a scoped name', () => { + expect(typesDirToSpecifier('babel__core')).to.eq('@babel/core'); + }); + it('should treat only the first separator as the scope separator', () => { + expect(typesDirToSpecifier('scope__pkg__name')).to.eq('@scope/pkg__name'); + }); +}); + +describe('globalVirtualStoreTypePaths()', () => { + let root: string; + const hoisted = () => path.join(root, 'node_modules', '.pnpm', 'node_modules'); + const write = (dir: string, manifest?: Record, files: string[] = []) => { + fs.ensureDirSync(dir); + if (manifest) fs.writeJsonSync(path.join(dir, 'package.json'), manifest); + files.forEach((file) => fs.outputFileSync(path.join(dir, file), '')); + }; + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'gvs-type-paths-')); + fs.ensureDirSync(path.join(root, 'node_modules')); + }); + afterEach(() => fs.removeSync(root)); + + it('should map a @types package whose runtime package ships no typings', () => { + write(path.join(root, 'node_modules', '@types', 'react'), { name: '@types/react' }); + write(path.join(root, 'node_modules', 'react'), { name: 'react', main: 'index.js' }); + expect(globalVirtualStoreTypePaths(root).react).to.deep.eq([path.join(root, 'node_modules', '@types', 'react')]); + }); + + it('should not map a package that ships typings through the types field', () => { + write(path.join(root, 'node_modules', '@types', 'glob'), { name: '@types/glob' }); + write(path.join(root, 'node_modules', 'glob'), { name: 'glob', types: './dist/index.d.ts' }); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('glob'); + }); + + it('should not map a package that ships an implicit index.d.ts', () => { + write(path.join(root, 'node_modules', '@types', 'chalk'), { name: '@types/chalk' }); + write(path.join(root, 'node_modules', 'chalk'), { name: 'chalk' }, ['index.d.ts']); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('chalk'); + }); + + it('should map a @types package whose runtime package is not installed at all', () => { + write(path.join(root, 'node_modules', '@types', 'node'), { name: '@types/node' }); + expect(globalVirtualStoreTypePaths(root)).to.have.property('node'); + }); + + it('should unmangle a scoped @types directory into its specifier', () => { + write(path.join(root, 'node_modules', '@types', 'babel__core'), { name: '@types/babel__core' }); + expect(globalVirtualStoreTypePaths(root)['@babel/core']).to.deep.eq([ + path.join(root, 'node_modules', '@types', 'babel__core'), + ]); + }); + + it('should let the hoisted directory win a specifier, as the walk out of .pnpm reached it first', () => { + write(path.join(hoisted(), '@types', 'semver'), { name: '@types/semver' }); + write(path.join(root, 'node_modules', '@types', 'semver'), { name: '@types/semver' }); + expect(globalVirtualStoreTypePaths(root).semver).to.deep.eq([path.join(hoisted(), '@types', 'semver')]); + }); + + it('should map @teambit only to the root, never to the hoisted copies of core aspects', () => { + write(path.join(hoisted(), '@teambit', 'harmony'), { name: '@teambit/harmony' }); + write(path.join(root, 'node_modules', '@teambit', 'harmony'), { name: '@teambit/harmony' }); + expect(globalVirtualStoreTypePaths(root)['@teambit/*']).to.deep.eq([ + path.join(root, 'node_modules', '@teambit', '*'), + ]); + }); + + it('should return nothing for a root with no node_modules', () => { + expect(globalVirtualStoreTypePaths(path.join(root, 'nope'))).to.deep.eq({}); + }); +}); + +describe('mergeTypePaths()', () => { + it('should keep a configured mapping over the bridged one', () => { + const merged = mergeTypePaths({ react: ['/configured'] }, { react: ['/bridged'], semver: ['/bridged'] }); + expect(merged).to.deep.eq({ react: ['/configured'], semver: ['/bridged'] }); + }); + it('should work with no configured paths at all', () => { + expect(mergeTypePaths(undefined, { react: ['/bridged'] })).to.deep.eq({ react: ['/bridged'] }); + }); +}); diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.ts new file mode 100644 index 000000000000..1b364b4163d7 --- /dev/null +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.ts @@ -0,0 +1,104 @@ +/** + * The type-resolution half of the hoisted-resolution bridge. + * + * `NODE_PATH` and the ESM loader in `@teambit/dependency-resolver`'s + * `hoisted-resolution-bridge` repair `require`/`import` for packages that live in pnpm's global + * virtual store. TypeScript reads neither, so under that layout a `.d.ts` in a store slot still + * cannot reach the workspace's `@types`: it resolves `react` to `react/index.js`, gets no + * typings, and every type derived from it silently degrades - props lose `children`, generics + * collapse to `unknown` - long before anything errors out at a place that names the cause. + * + * TypeScript's only lever here is `paths`, which is a *redirect* and not the fallback `NODE_PATH` + * is, so a blanket mapping does real damage: pointing every `@types` package at the bridge + * directories overrides packages that ship their own typings, and the resolution that used to be + * a last resort starts winning over a package's declared, newer types. This module maps only what + * the walk used to find and nothing else: + * + * - a `@types/x` mapping only when `x` itself ships no typings. That is the case the walk-up + * existed for; when `x` is self-typed, the walk stopped at `x` and never reached `@types/x`. + * - `@teambit/*` from the root's own `node_modules`, never the hoisted directory. Core aspects + * have to be the single copy from the running installation (the same invariant + * `DependencyLinker` maintains), and the hoisted directory holds transitive - older - copies + * that would otherwise win. + */ +import * as fs from 'fs'; +import * as path from 'path'; +import { hoistedResolutionDirs } from '@teambit/dependency-resolver'; + +export type TypePaths = Record; + +/** + * `@types/react` -> `react`, `@types/babel__core` -> `@babel/core`. The double underscore is + * DefinitelyTyped's mangling for a scope separator, and only the first one is the separator - a + * package whose own name contains `__` keeps it. + */ +export function typesDirToSpecifier(dirName: string): string { + const separator = dirName.indexOf('__'); + if (separator === -1) return dirName; + return `@${dirName.slice(0, separator)}/${dirName.slice(separator + 2)}`; +} + +/** + * Whether the package the specifier names ships typings of its own, judged at the first bridge + * directory that holds it - the same copy the resolver reaches. A package that is not installed + * at either counts as untyped: nothing shadows the `@types` mapping in that case. + */ +function shipsOwnTypes(specifier: string, dirs: string[]): boolean { + for (const dir of dirs) { + const packageDir = path.join(dir, specifier); + let manifest: { types?: unknown; typings?: unknown }; + try { + manifest = JSON.parse(fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8')); + } catch { + continue; // not in this directory - keep looking in the next + } + if (typeof manifest.types === 'string' || typeof manifest.typings === 'string') return true; + // the implicit form: no `types` field, but an index.d.ts beside the entry point + return fs.existsSync(path.join(packageDir, 'index.d.ts')); + } + return false; +} + +/** + * `paths` entries that let a store slot resolve the types it used to reach by walking up out of + * `node_modules/.pnpm`. Empty for a root that is not on the global virtual store - callers gate on + * `isGlobalVirtualStoreLayout` - or one whose directories no longer exist. + * + * Reads the directories on every call rather than caching them: an install can move a workspace + * onto the global virtual store mid-process, and the envs compiled right after it would otherwise + * be handed the layout from before. + */ +export function globalVirtualStoreTypePaths(root: string): TypePaths { + const dirs = hoistedResolutionDirs(root); + if (!dirs.length) return {}; + const paths: TypePaths = {}; + for (const dir of dirs) { + const typesDir = path.join(dir, '@types'); + let entries: string[]; + try { + entries = fs.readdirSync(typesDir); + } catch { + continue; + } + for (const entry of entries) { + const specifier = typesDirToSpecifier(entry); + // walk order decides: the hoisted directory is reached first, so it keeps the specifier + if (paths[specifier] || shipsOwnTypes(specifier, dirs)) continue; + paths[specifier] = [path.join(typesDir, entry)]; + } + } + const coreAspects = path.join(root, 'node_modules', '@teambit'); + if (fs.existsSync(coreAspects)) { + paths['@teambit/*'] = [path.join(coreAspects, '*')]; + } + return paths; +} + +/** + * Merge bridge entries under whatever the env and the component already configured: a specifier + * someone mapped deliberately keeps its mapping, and the bridge only fills in what would + * otherwise resolve to nothing. + */ +export function mergeTypePaths(configured: TypePaths | undefined, bridged: TypePaths): TypePaths { + return { ...bridged, ...configured }; +} diff --git a/scopes/typescript/typescript/typescript.main.runtime.ts b/scopes/typescript/typescript/typescript.main.runtime.ts index 3146deedf61e..5920e1e9bfb8 100644 --- a/scopes/typescript/typescript/typescript.main.runtime.ts +++ b/scopes/typescript/typescript/typescript.main.runtime.ts @@ -13,7 +13,12 @@ import { TypescriptConfigMutator } from '@teambit/typescript.modules.ts-config-m import { WorkspaceAspect } from '@teambit/workspace'; import type { Workspace } from '@teambit/workspace'; import type { DependencyResolverMain } from '@teambit/dependency-resolver'; -import { DependencyResolverAspect } from '@teambit/dependency-resolver'; +import { + DependencyResolverAspect, + isGlobalVirtualStoreLayout, + selfInstallationRoot, +} from '@teambit/dependency-resolver'; +import { globalVirtualStoreTypePaths, mergeTypePaths } from './global-virtual-store-type-paths'; import pMapSeries from 'p-map-series'; import type { TsserverClientOpts } from '@teambit/ts-server'; import { TsserverClient } from '@teambit/ts-server'; @@ -136,11 +141,38 @@ export class TypescriptMain { TypescriptAspect.id, this.logger, afterMutationWithoutTsconfig, - afterMutation.raw.tsconfig, + this.bridgeTypeResolution(afterMutation.raw.tsconfig), tsModule as any ); } + /** + * Give the compiled program the types a store slot can no longer reach by walking up. Applied + * last, after every transformer, so it sees the final `paths` and yields to anything configured + * there. See `./global-virtual-store-type-paths`. + * + * Two roots, each gated on the layout its own last install recorded, and both for the same + * reason the runtime bridge covers both: the workspace, whose components are what gets compiled, + * and the running installation, whose core aspects the compiled program reaches through the + * links `DependencyLinker` writes. A bvm installation is project-local and stays out. + */ + private bridgeTypeResolution(tsconfig: any): any { + const roots = [this.workspace?.path, selfInstallationRoot()].filter( + (root, index, all): root is string => Boolean(root) && all.indexOf(root) === index + ); + const bridged = roots + .filter((root) => isGlobalVirtualStoreLayout(root)) + .map((root) => globalVirtualStoreTypePaths(root)); + if (!bridged.length) return tsconfig; + // workspace first: where both roots map a specifier, the workspace's copy is the one its + // components already compile against + const merged = bridged.reduceRight((acc, paths) => ({ ...acc, ...paths }), {}); + if (!Object.keys(merged).length) return tsconfig; + const compilerOptions = { ...tsconfig?.compilerOptions }; + compilerOptions.paths = mergeTypePaths(compilerOptions.paths, merged); + return { ...tsconfig, compilerOptions }; + } + /** * get TsserverClient instance if initiated already, otherwise, return undefined. */ From 2f834311e4b452be6724d0acb3e53288cd233f3a Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 11:44:37 +0200 Subject: [PATCH 2/8] fix(global virtual store): count every form a package ships types in A false negative in shipsOwnTypes is not a missed optimization: the specifier gets redirected to @types and TypeScript stops seeing the package's own, usually newer, declarations. The check read the types/typings fields and a root index.d.ts, which misses a package that says nothing and lets the resolver infer dist/index.d.ts from its entry point, one that declares a types condition inside exports, and one that maps declarations through typesVersions. Co-Authored-By: Claude Opus 5 (1M context) --- .../global-virtual-store-type-paths.spec.ts | 36 +++++++++++++++ .../global-virtual-store-type-paths.ts | 45 +++++++++++++++++-- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts index 997f688e7ccf..6dff4ebc3a25 100644 --- a/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts @@ -49,6 +49,42 @@ describe('globalVirtualStoreTypePaths()', () => { expect(globalVirtualStoreTypePaths(root)).to.not.have.property('chalk'); }); + it('should not map a package whose declarations sit beside a nested entry point', () => { + write(path.join(root, 'node_modules', '@types', 'nested'), { name: '@types/nested' }); + write(path.join(root, 'node_modules', 'nested'), { name: 'nested', main: 'dist/index.js' }, ['dist/index.d.ts']); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('nested'); + }); + + it('should not map a package whose entry point is a directory with an index.d.ts', () => { + write(path.join(root, 'node_modules', '@types', 'dir-entry'), { name: '@types/dir-entry' }); + write(path.join(root, 'node_modules', 'dir-entry'), { name: 'dir-entry', main: './lib' }, ['lib/index.d.ts']); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('dir-entry'); + }); + + it('should not map a package that declares types through a conditional export', () => { + write(path.join(root, 'node_modules', '@types', 'exported'), { name: '@types/exported' }); + write(path.join(root, 'node_modules', 'exported'), { + name: 'exported', + exports: { '.': { import: { types: './dist/index.d.ts', default: './dist/index.js' } } }, + }); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('exported'); + }); + + it('should not map a package that ships types through typesVersions', () => { + write(path.join(root, 'node_modules', '@types', 'versioned'), { name: '@types/versioned' }); + write(path.join(root, 'node_modules', 'versioned'), { + name: 'versioned', + typesVersions: { '>=4.0': { '*': ['types/*'] } }, + }); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('versioned'); + }); + + it('should still map a package that only ships javascript beside its entry point', () => { + write(path.join(root, 'node_modules', '@types', 'plain'), { name: '@types/plain' }); + write(path.join(root, 'node_modules', 'plain'), { name: 'plain', main: 'dist/index.js' }, ['dist/index.js']); + expect(globalVirtualStoreTypePaths(root)).to.have.property('plain'); + }); + it('should map a @types package whose runtime package is not installed at all', () => { write(path.join(root, 'node_modules', '@types', 'node'), { name: '@types/node' }); expect(globalVirtualStoreTypePaths(root)).to.have.property('node'); diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.ts index 1b364b4163d7..7c3668e34b56 100644 --- a/scopes/typescript/typescript/global-virtual-store-type-paths.ts +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.ts @@ -38,23 +38,62 @@ export function typesDirToSpecifier(dirName: string): string { return `@${dirName.slice(0, separator)}/${dirName.slice(separator + 2)}`; } +type PackageManifest = { + types?: unknown; + typings?: unknown; + typesVersions?: unknown; + exports?: unknown; + main?: unknown; +}; + +/** Whether an `exports` map declares a `types` condition anywhere in its conditional nesting. */ +function exportsDeclareTypes(exportsField: unknown): boolean { + if (!exportsField || typeof exportsField !== 'object') return false; + return Object.entries(exportsField as Record).some( + ([condition, target]) => condition === 'types' || exportsDeclareTypes(target) + ); +} + +/** + * The declaration file TypeScript infers from an entry point, which is how a package ships types + * without saying so: `dist/index.js` is typed by `dist/index.d.ts`, and a directory entry point by + * an `index.d.ts` inside it. + */ +function declarationsBesideEntry(packageDir: string, main: unknown): boolean { + const entry = typeof main === 'string' && main ? main : 'index.js'; + const resolved = path.join(packageDir, entry); + const withoutExtension = resolved.replace(/\.(js|cjs|mjs|jsx)$/, ''); + return ( + fs.existsSync(`${withoutExtension}.d.ts`) || + fs.existsSync(path.join(resolved, 'index.d.ts')) || + fs.existsSync(path.join(packageDir, 'index.d.ts')) + ); +} + /** * Whether the package the specifier names ships typings of its own, judged at the first bridge * directory that holds it - the same copy the resolver reaches. A package that is not installed * at either counts as untyped: nothing shadows the `@types` mapping in that case. + * + * Every form a package can ship types in has to count, because a false negative here is not a + * missed optimization - it redirects a specifier to `@types` and *overrides* the package's own, + * usually newer, declarations. `types`/`typings`, a `types` condition in `exports`, + * `typesVersions`, and the declaration file inferred from the entry point all mean the same thing + * to the resolver, which reached the package itself and never looked at `@types` at all. */ function shipsOwnTypes(specifier: string, dirs: string[]): boolean { for (const dir of dirs) { const packageDir = path.join(dir, specifier); - let manifest: { types?: unknown; typings?: unknown }; + let manifest: PackageManifest; try { manifest = JSON.parse(fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8')); } catch { continue; // not in this directory - keep looking in the next } if (typeof manifest.types === 'string' || typeof manifest.typings === 'string') return true; - // the implicit form: no `types` field, but an index.d.ts beside the entry point - return fs.existsSync(path.join(packageDir, 'index.d.ts')); + if (manifest.typesVersions && typeof manifest.typesVersions === 'object') return true; + if (exportsDeclareTypes(manifest.exports)) return true; + return declarationsBesideEntry(packageDir, manifest.main); } return false; } From d539648e0cbefb7505a90496596fe368fee0b72e Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 12:16:30 +0200 Subject: [PATCH 3/8] fix(global virtual store): rebuild NODE_PATH instead of prepending what is missing Order in NODE_PATH is resolution order, and an entry already present is not necessarily in front of the one it has to beat. A bit whose bridge added the hoisted directory alone leaves it in NODE_PATH for its children, where adding the root's node_modules in front of it inverts the walk the bridge exists to reproduce. Rebuild the two entries at the front in walk order and keep everything else behind them, unchanged in relative order. Co-Authored-By: Claude Opus 5 (1M context) --- .../hoisted-resolution-bridge.spec.ts | 69 ++++++++++++++++++- .../hoisted-resolution-bridge.ts | 15 ++-- 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts index 0024bf0276c6..127d9cd7b795 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts @@ -2,7 +2,12 @@ import { expect } from 'chai'; import fs from 'fs-extra'; import os from 'os'; import path from 'path'; -import { hoistedResolutionDirs, isPathInsideOrEqual, parseRecordedVirtualStoreDir } from './hoisted-resolution-bridge'; +import { + ensureHoistedDependencyResolution, + hoistedResolutionDirs, + isPathInsideOrEqual, + parseRecordedVirtualStoreDir, +} from './hoisted-resolution-bridge'; describe('isPathInsideOrEqual()', () => { const base = path.resolve('/base'); @@ -61,3 +66,65 @@ describe('hoistedResolutionDirs()', () => { expect(hoistedResolutionDirs(root)).to.deep.eq([]); }); }); + +describe('ensureHoistedDependencyResolution()', () => { + let root: string; + let nodePath: string | undefined; + let nodeOptions: string | undefined; + 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; + }); + 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; + 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 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); + } + }); +}); diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts index 1e27e5e8b593..7aab39726e83 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts @@ -230,12 +230,15 @@ export function ensureHoistedDependencyResolution(workspaceRoot: string): void { const dirs = hoistedResolutionDirs(workspaceRoot); if (!dirs.length) return; const existing = process.env.NODE_PATH; - const known = new Set(existing?.split(path.delimiter).filter(Boolean) ?? []); - const added = dirs.filter((dir) => !known.has(dir)); - if (added.length) { - // prepended in walk order, so the hoisted directory keeps winning over the root's own - // node_modules exactly as the walk out of `.pnpm` used to reach it first - process.env.NODE_PATH = [...added, ...(existing ? [existing] : [])].join(path.delimiter); + const untouched = (existing?.split(path.delimiter).filter(Boolean) ?? []).filter((dir) => !dirs.includes(dir)); + // rebuilt rather than prepended, because order is resolution order and an entry already present + // is not necessarily in front of the one it has to beat: a bit that bridged the hoisted + // directory alone leaves it in `NODE_PATH` for its children, and adding the root's node_modules + // in front of it there would invert the walk the bridge exists to reproduce. Everything this + // bridge does not own keeps its relative order, behind what the walk reached first. + const next = [...dirs, ...untouched].join(path.delimiter); + if (next !== existing) { + process.env.NODE_PATH = next; // `NODE_PATH` is read once when the module system initializes, so a later assignment only // takes effect after re-deriving the global paths. (require('module') as { _initPaths(): void })._initPaths(); From fa00cbddecd85bc9ba236a762132194d0b22a8e8 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 12:29:12 +0200 Subject: [PATCH 4/8] test(global virtual store): keep the bridge's process-global effects inside the test ensureHoistedDependencyResolution mutates state no afterEach can reach by restoring an environment variable: _initPaths rederives Module.globalPaths, and module.register installs an ESM loader for the life of the process. The cases restored NODE_PATH and left the resolver pointing into directories they then deleted, and each of them chained another loader. Rederive the paths from the restored variable, and take `register` away for the duration so the irreversible half never runs - through the same guard that carries runtimes without it. These cases are about NODE_PATH order. Co-Authored-By: Claude Opus 5 (1M context) --- .../hoisted-resolution-bridge.spec.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts index 127d9cd7b795..0cb9a012cb3b 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts @@ -1,5 +1,6 @@ import { expect } from 'chai'; import fs from 'fs-extra'; +import Module from 'module'; import os from 'os'; import path from 'path'; import { @@ -71,6 +72,11 @@ 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); @@ -80,12 +86,21 @@ describe('ensureHoistedDependencyResolution()', () => { 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); }); From a9abe6a5feea1528e4cd9ea2ca1785746a5e056a Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 12:39:19 +0200 Subject: [PATCH 5/8] fix(global virtual store): recognise declarations under every entry point shape TypeScript pairs .mjs with .d.mts and .cjs with .d.cts, and infers a package's declarations from whichever entry point the resolver picked - main for the classic algorithm, an exports target for the modern one. The check looked only for .d.ts beside main, so a package shipping types either of those ways read as untyped and had its specifier redirected to @types, burying the declarations it actually ships. Co-Authored-By: Claude Opus 5 (1M context) --- .../global-virtual-store-type-paths.spec.ts | 28 ++++++++++++++++ .../global-virtual-store-type-paths.ts | 33 +++++++++++++++---- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts index 6dff4ebc3a25..7cbcffde4e6c 100644 --- a/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts @@ -61,6 +61,34 @@ describe('globalVirtualStoreTypePaths()', () => { expect(globalVirtualStoreTypePaths(root)).to.not.have.property('dir-entry'); }); + it('should not map a package whose declarations carry the esm extension', () => { + write(path.join(root, 'node_modules', '@types', 'esm-typed'), { name: '@types/esm-typed' }); + write(path.join(root, 'node_modules', 'esm-typed'), { name: 'esm-typed', main: 'dist/index.mjs' }, [ + 'dist/index.d.mts', + ]); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('esm-typed'); + }); + + it('should not map a package whose declarations carry the cjs extension', () => { + write(path.join(root, 'node_modules', '@types', 'cjs-typed'), { name: '@types/cjs-typed' }); + write(path.join(root, 'node_modules', 'cjs-typed'), { name: 'cjs-typed', main: 'dist/index.cjs' }, [ + 'dist/index.d.cts', + ]); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('cjs-typed'); + }); + + it('should not map a package whose declarations sit beside an export target', () => { + // no `types` condition and no `main`: the modern resolver infers the declarations from the + // target it picked, and so does this + write(path.join(root, 'node_modules', '@types', 'export-typed'), { name: '@types/export-typed' }); + write(path.join(root, 'node_modules', 'export-typed'), { + name: 'export-typed', + exports: { '.': { import: './dist/index.mjs' } }, + }); + fs.outputFileSync(path.join(root, 'node_modules', 'export-typed', 'dist', 'index.d.mts'), ''); + expect(globalVirtualStoreTypePaths(root)).to.not.have.property('export-typed'); + }); + it('should not map a package that declares types through a conditional export', () => { write(path.join(root, 'node_modules', '@types', 'exported'), { name: '@types/exported' }); write(path.join(root, 'node_modules', 'exported'), { diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.ts index 7c3668e34b56..a8f75225618e 100644 --- a/scopes/typescript/typescript/global-virtual-store-type-paths.ts +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.ts @@ -54,19 +54,36 @@ function exportsDeclareTypes(exportsField: unknown): boolean { ); } +/** Every file an `exports` map points at, at any depth of subpath and condition nesting. */ +function exportTargets(exportsField: unknown, collected: string[] = []): string[] { + if (typeof exportsField === 'string') collected.push(exportsField); + else if (exportsField && typeof exportsField === 'object') { + Object.values(exportsField as Record).forEach((target) => exportTargets(target, collected)); + } + return collected; +} + +/** Every extension a declaration file can carry: `.mjs` is typed by `.d.mts`, `.cjs` by `.d.cts`. */ +const DECLARATION_EXTENSIONS = ['.d.ts', '.d.mts', '.d.cts']; + /** * The declaration file TypeScript infers from an entry point, which is how a package ships types * without saying so: `dist/index.js` is typed by `dist/index.d.ts`, and a directory entry point by * an `index.d.ts` inside it. + * + * All three extensions are tried against each shape rather than the one paired with the entry's + * own extension. Being too eager here only skips a mapping; missing one redirects the specifier to + * `@types` and buries the declarations the package actually ships. */ function declarationsBesideEntry(packageDir: string, main: unknown): boolean { const entry = typeof main === 'string' && main ? main : 'index.js'; const resolved = path.join(packageDir, entry); const withoutExtension = resolved.replace(/\.(js|cjs|mjs|jsx)$/, ''); - return ( - fs.existsSync(`${withoutExtension}.d.ts`) || - fs.existsSync(path.join(resolved, 'index.d.ts')) || - fs.existsSync(path.join(packageDir, 'index.d.ts')) + return DECLARATION_EXTENSIONS.some( + (extension) => + fs.existsSync(`${withoutExtension}${extension}`) || + fs.existsSync(path.join(resolved, `index${extension}`)) || + fs.existsSync(path.join(packageDir, `index${extension}`)) ); } @@ -78,8 +95,9 @@ function declarationsBesideEntry(packageDir: string, main: unknown): boolean { * Every form a package can ship types in has to count, because a false negative here is not a * missed optimization - it redirects a specifier to `@types` and *overrides* the package's own, * usually newer, declarations. `types`/`typings`, a `types` condition in `exports`, - * `typesVersions`, and the declaration file inferred from the entry point all mean the same thing - * to the resolver, which reached the package itself and never looked at `@types` at all. + * `typesVersions`, and the declaration file inferred from an entry point - `main` for the classic + * resolver, any `exports` target for the modern one - all mean the same thing to the resolver, + * which reached the package itself and never looked at `@types` at all. */ function shipsOwnTypes(specifier: string, dirs: string[]): boolean { for (const dir of dirs) { @@ -93,7 +111,8 @@ function shipsOwnTypes(specifier: string, dirs: string[]): boolean { if (typeof manifest.types === 'string' || typeof manifest.typings === 'string') return true; if (manifest.typesVersions && typeof manifest.typesVersions === 'object') return true; if (exportsDeclareTypes(manifest.exports)) return true; - return declarationsBesideEntry(packageDir, manifest.main); + const entries = [manifest.main, ...exportTargets(manifest.exports)]; + return entries.some((entry) => declarationsBesideEntry(packageDir, entry)); } return false; } From f2450a096ce243b9eaeacac935591afa5a3eeeb2 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 12:49:48 +0200 Subject: [PATCH 6/8] fix(global virtual store): compare NODE_PATH entries by path, not by spelling An entry naming a directory the bridge owns, written in another spelling - a trailing separator, a redundant segment, a relative path, or on Windows a difference in case - was treated as foreign and kept alongside the canonical entry the bridge then added. path.relative normalizes both sides, and the module already reasons about platform path comparison that way for containment. Co-Authored-By: Claude Opus 5 (1M context) --- .../hoisted-resolution-bridge.spec.ts | 20 +++++++++++++++++++ .../hoisted-resolution-bridge.ts | 16 ++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts index 0cb9a012cb3b..dc216c10c251 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts @@ -7,6 +7,7 @@ import { ensureHoistedDependencyResolution, hoistedResolutionDirs, isPathInsideOrEqual, + isSamePath, parseRecordedVirtualStoreDir, } from './hoisted-resolution-bridge'; @@ -125,6 +126,12 @@ describe('ensureHoistedDependencyResolution()', () => { 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; @@ -143,3 +150,16 @@ describe('ensureHoistedDependencyResolution()', () => { } }); }); + +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); + }); +}); diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts index 7aab39726e83..1eb656c3fe75 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts @@ -137,6 +137,16 @@ export function isPathInsideOrEqual(child: string, parent: string): boolean { return !path.isAbsolute(rel) && rel !== '..' && !rel.startsWith(`..${path.sep}`); } +/** + * Whether two spellings name the same directory. `path.relative` normalizes both sides first, so a + * trailing separator, a redundant `.`, a relative spelling and - on Windows - a difference in case + * all compare equal, which string equality would miss. Anything written by hand into `NODE_PATH` + * arrives in whatever spelling its author chose. + */ +export function isSamePath(one: string, other: string): boolean { + return path.relative(one, other) === ''; +} + export function isGlobalVirtualStoreLayout(workspaceRoot: string): boolean { let modulesManifest: string; try { @@ -230,7 +240,11 @@ export function ensureHoistedDependencyResolution(workspaceRoot: string): void { const dirs = hoistedResolutionDirs(workspaceRoot); if (!dirs.length) return; const existing = process.env.NODE_PATH; - const untouched = (existing?.split(path.delimiter).filter(Boolean) ?? []).filter((dir) => !dirs.includes(dir)); + const untouched = (existing?.split(path.delimiter).filter(Boolean) ?? []).filter( + // by path rather than by spelling, so an entry naming a directory this bridge owns is replaced + // by the canonical one instead of being kept alongside it as a redundant search base + (dir) => !dirs.some((owned) => isSamePath(dir, owned)) + ); // rebuilt rather than prepended, because order is resolution order and an entry already present // is not necessarily in front of the one it has to beat: a bit that bridged the hoisted // directory alone leaves it in `NODE_PATH` for its children, and adding the root's node_modules From f5d7435c549dfa9ee98fad9b642ebaa41e60e915 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 14:01:52 +0200 Subject: [PATCH 7/8] docs(global virtual store): state where the layout check belongs globalVirtualStoreTypePaths read as though it returned nothing for a project-local root, which it has no way of knowing - it derives the mapping from the directories alone. Say what it does, and why the check sits at the call site: a project-local root would get a mapping pointing at the same @types its own walk already reaches, and the caller is the one holding several roots and deciding which of them participate. Co-Authored-By: Claude Opus 5 (1M context) --- .../typescript/global-virtual-store-type-paths.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scopes/typescript/typescript/global-virtual-store-type-paths.ts b/scopes/typescript/typescript/global-virtual-store-type-paths.ts index a8f75225618e..249399b12101 100644 --- a/scopes/typescript/typescript/global-virtual-store-type-paths.ts +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.ts @@ -119,8 +119,14 @@ function shipsOwnTypes(specifier: string, dirs: string[]): boolean { /** * `paths` entries that let a store slot resolve the types it used to reach by walking up out of - * `node_modules/.pnpm`. Empty for a root that is not on the global virtual store - callers gate on - * `isGlobalVirtualStoreLayout` - or one whose directories no longer exist. + * `node_modules/.pnpm`. Derived purely from {@link hoistedResolutionDirs}, so a root whose + * directories are gone yields an empty mapping. + * + * The layout is the caller's to check. This reads no `.modules.yaml` and asks no questions about + * the virtual store: a project-local root would get a mapping too, pointing at the same `@types` + * its own walk already reaches - unnecessary rather than wrong, but unnecessary is reason enough + * to keep it out. Callers decide per root, gating on `isGlobalVirtualStoreLayout`, because they + * are the ones holding several roots and deciding which of them participate. * * Reads the directories on every call rather than caching them: an install can move a workspace * onto the global virtual store mid-process, and the envs compiled right after it would otherwise From 82540b43537b112d8f9c9377d05cbb7d3e17b092 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Aug 2026 14:18:57 +0200 Subject: [PATCH 8/8] fix(global virtual store): re-register the esm loader when the order changes The loader inlines its entry list and searches it in order, so a reordered NODE_PATH needs a new registration - but the skip test asked whether the entry *set* had grown, which a reorder leaves untouched. Children then inherit an --import flag whose precedence contradicts the NODE_PATH beside it. Track the ordered list instead of the set. In-process the correction is partial by construction, and the comments now say so: node runs the last registered loader first and it delegates before its own fallback, so an earlier registration keeps precedence for entries it already had. Co-Authored-By: Claude Opus 5 (1M context) --- .../hoisted-resolution-bridge.spec.ts | 50 +++++++++++++++++++ .../hoisted-resolution-bridge.ts | 32 ++++++++---- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts index dc216c10c251..2a2f02e25d35 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts @@ -151,6 +151,56 @@ describe('ensureHoistedDependencyResolution()', () => { }); }); +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', () => { diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts index 1eb656c3fe75..b19a7e81a086 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts +++ b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts @@ -36,10 +36,18 @@ * old layout, yet the same process goes on to reload envs and compile from store slots. The * installer re-applies the bridge the moment the target layout is known. * - * Everything here is idempotent, so calling it again is always safe: each `NODE_PATH` entry is - * added once, the ESM loader re-registers only when the entry set grows (the previous, subset - * loader keeps chaining harmlessly), and the process's own `NODE_OPTIONS` flag is replaced - * rather than stacked. + * Everything here is idempotent, so calling it again is always safe: `NODE_PATH` is rebuilt to the + * same value, the ESM loader re-registers only when the ordered entry list changes, and the + * process's own `NODE_OPTIONS` flag is replaced rather than stacked. + * + * One asymmetry to know about when reading the ESM half. Node runs the *last* registered loader + * first and it delegates through `defaultResolve` before its own fallback, so an earlier + * registration resolves anything it can before a later one is consulted. A re-registration + * therefore corrects what child processes inherit - the `--import` flag carries the current list - + * while in-process the earlier registration keeps its precedence for the entries it already had. + * That only diverges from CommonJS for a specifier resolvable from more than one entry across two + * registrations with different orders, which needs a process bridging two roots in alternating + * order; the CommonJS half, rebuilt on every call, is always right. */ import * as fs from 'fs'; import * as path from 'path'; @@ -50,8 +58,9 @@ import { pathToFileURL } from 'url'; * The entries cannot be read from `NODE_PATH` inside the loader: on modern Node the hooks run on * a separate thread whose `process.env` is a snapshot taken at registration, so late additions - * an install switching the workspace onto the global virtual store mid-process - would never be - * seen. Injecting the list makes each registration self-contained; when the set grows, a new - * loader is registered and chains after the previous (whose entries are a subset - harmless). + * seen. Injecting the list makes each registration self-contained, at the cost of needing a new + * registration whenever the list changes - by order as much as by membership, since the loader + * searches these entries in the order given. */ const esmNodePathLoaderSource = (dirs: string[]) => ` import { createRequire } from 'node:module' @@ -91,7 +100,7 @@ export async function resolve (specifier, context, defaultResolve) { } `; -const registeredEsmDirs = new Set(); +let lastRegisteredEntries: string | undefined; let lastImportFlag: string | undefined; /** @@ -281,10 +290,13 @@ function registerEsmNodePathLoader(): void { register?: (specifier: string, parentURL: string) => void; }; if (typeof nodeModule.register !== 'function') return; - // mirror the CommonJS side exactly: every current NODE_PATH entry participates + // mirror the CommonJS side exactly: every current NODE_PATH entry participates, in its order const dirs = (process.env.NODE_PATH || '').split(path.delimiter).filter(Boolean); if (!dirs.length) return; - if (lastImportFlag && dirs.every((dir) => registeredEsmDirs.has(dir))) return; + const entries = dirs.join(path.delimiter); + // ordered, not a set: the loader inlines the list, so a reordering leaves a registration whose + // precedence no longer matches the one CommonJS now uses + if (lastImportFlag && entries === lastRegisteredEntries) return; const loaderUrl = `data:text/javascript,${encodeURIComponent(esmNodePathLoaderSource(dirs))}`; const parentUrl = pathToFileURL(path.join(process.cwd(), '/')).href; try { @@ -294,7 +306,7 @@ function registerEsmNodePathLoader(): void { // half of the workaround is unaffected, and only ESM packages relying on hoisting are lost. return; } - dirs.forEach((dir) => registeredEsmDirs.add(dir)); + lastRegisteredEntries = entries; const registration = `import{register}from'node:module';register(${JSON.stringify(loaderUrl)},${JSON.stringify(parentUrl)});`; const importFlag = `--import=data:text/javascript,${encodeURIComponent(registration)}`; // children get one flag carrying the full current set: replace our previous flag rather than