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..2a2f02e25d35 100644 --- a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts +++ b/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); + }); +}); diff --git a/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts b/scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts index 640fdca6e417..b19a7e81a086 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,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: the `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'; @@ -43,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' @@ -69,9 +85,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) @@ -83,7 +100,7 @@ export async function resolve (specifier, context, defaultResolve) { } `; -const registeredEsmDirs = new Set(); +let lastRegisteredEntries: string | undefined; let lastImportFlag: string | undefined; /** @@ -129,6 +146,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 { @@ -167,7 +194,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 +209,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 +224,44 @@ 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 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 + // 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(); @@ -236,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 { @@ -249,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 @@ -258,9 +315,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..7cbcffde4e6c --- /dev/null +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts @@ -0,0 +1,155 @@ +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 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 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'), { + 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'); + }); + + 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..249399b12101 --- /dev/null +++ b/scopes/typescript/typescript/global-virtual-store-type-paths.ts @@ -0,0 +1,168 @@ +/** + * 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)}`; +} + +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) + ); +} + +/** 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 DECLARATION_EXTENSIONS.some( + (extension) => + fs.existsSync(`${withoutExtension}${extension}`) || + fs.existsSync(path.join(resolved, `index${extension}`)) || + fs.existsSync(path.join(packageDir, `index${extension}`)) + ); +} + +/** + * 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 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) { + const packageDir = path.join(dir, specifier); + 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; + if (manifest.typesVersions && typeof manifest.typesVersions === 'object') return true; + if (exportsDeclareTypes(manifest.exports)) return true; + const entries = [manifest.main, ...exportTargets(manifest.exports)]; + return entries.some((entry) => declarationsBesideEntry(packageDir, entry)); + } + return false; +} + +/** + * `paths` entries that let a store slot resolve the types it used to reach by walking up out of + * `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 + * 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. */