From 87c1e0542046d9376d08f8d3300ba1a2b76b9d30 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 10 Aug 2026 22:09:42 -0400 Subject: [PATCH] chore(deps): count triple-slash type references as candidates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From Copilot's twelfth pass of #1962. A `/// ` pulls in a package's declarations exactly like an import does, but TypeScript reports it in `preProcessFile`'s `typeReferenceDirectives`, not `importedFiles` — and only the latter was read. A shared source using one could therefore load the root's copy of a package alongside a client's without it ever entering the candidate set, so its skew would pass. Same silent-miss direction as the earlier syntax gaps, through a channel the parser switch didn't cover because I only read one of its two outputs. `typeReferencePackageNames` maps a directive to both candidates, since the name is the *type* name and not the package: `node` resolves to `@types/node`, while a package shipping its own declarations resolves to itself. Returning both over-approximates, the safe direction — whichever isn't installed drops out downstream. Scoped names mangle `@scope/pkg` → `@types/scope__pkg`, TypeScript's convention. `path` references name a file, not a package, and are ignored. The real candidate set is unchanged at 17: no shared source uses a triple-slash reference today. As with the other syntax fixes, this closes the channel against the one that arrives later. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 --- scripts/verify-dep-lockstep.mjs | 30 +++++++++++++++++++++-- scripts/verify-dep-lockstep.test.mjs | 36 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/scripts/verify-dep-lockstep.mjs b/scripts/verify-dep-lockstep.mjs index ab928835..22ebb9ad 100644 --- a/scripts/verify-dep-lockstep.mjs +++ b/scripts/verify-dep-lockstep.mjs @@ -175,7 +175,30 @@ function typescript() { } /** - * Every third-party package name imported by a blob of TypeScript source. + * The package(s) a `/// ` directive can resolve to + * (Copilot, #1962). Such a directive pulls in declarations exactly like an + * import does, but TypeScript reports it separately from `importedFiles`, so + * reading only the latter would let a referenced package skew unseen. + * + * Both candidates are returned because the directive name is the *type* name, + * not the package: `node` resolves to `@types/node`, while a package shipping + * its own declarations resolves to itself. Returning both over-approximates, + * which is the safe direction — whichever isn't installed drops out. A scoped + * name mangles as `@scope/pkg` → `@types/scope__pkg`, TypeScript's convention. + */ +export function typeReferencePackageNames(directive) { + const name = packageNameOf(directive); + if (!name) return []; + const scoped = /^@([^/]+)\/(.+)$/.exec(name); + const typesName = scoped + ? `@types/${scoped[1]}__${scoped[2]}` + : `@types/${name}`; + return [name, typesName]; +} + +/** + * Every third-party package name whose declarations a blob of TypeScript source + * pulls in — via an import of any form, or a triple-slash type reference. * Over-approximating is safe (a name absent from every lockfile contributes * nothing downstream — `@inspector/core` is a build-time alias, not a package, * and drops out that way); under-approximating is not, since a missed package @@ -185,11 +208,14 @@ export function importedPackageNames(source) { const names = new Set(); // (source, readImportFiles, detectJavaScriptImports) — the latter two make it // report `require(…)` and dynamic imports as well as static ones. - const { importedFiles } = typescript().preProcessFile(source, true, true); + const { importedFiles, typeReferenceDirectives } = + typescript().preProcessFile(source, true, true); for (const { fileName } of importedFiles) { const name = packageNameOf(fileName); if (name) names.add(name); } + for (const { fileName } of typeReferenceDirectives ?? []) + for (const name of typeReferencePackageNames(fileName)) names.add(name); return names; } diff --git a/scripts/verify-dep-lockstep.test.mjs b/scripts/verify-dep-lockstep.test.mjs index 79974404..a00ce642 100644 --- a/scripts/verify-dep-lockstep.test.mjs +++ b/scripts/verify-dep-lockstep.test.mjs @@ -16,6 +16,7 @@ import { partitionSkew, sourcesWithNoFiles, topLevelLockVersions, + typeReferencePackageNames, } from "./verify-dep-lockstep.mjs"; test("isSharedSourceFile: all four TS extensions, not just .ts/.tsx", () => { @@ -196,6 +197,41 @@ test("importedPackageNames: static, side-effect, and dynamic forms; builtins and ]); }); +test("importedPackageNames: triple-slash type references count (Copilot, #1962)", () => { + // A `/// ` pulls in declarations exactly like an + // import, but TypeScript reports it in `typeReferenceDirectives`, not + // `importedFiles` — so reading only the latter let a referenced package skew + // unseen. `path` references name a file, not a package, and are ignored. + const source = [ + '/// ', + '/// ', + '/// ', + 'import { z } from "zod";', + ].join("\n"); + assert.deepEqual([...importedPackageNames(source)].sort(), [ + "@types/express", + "@types/node", + "express", + "node", + "zod", + ]); +}); + +test("typeReferencePackageNames: both the bare and the @types form (Copilot, #1962)", () => { + // The directive names a *type*, not a package: `node` resolves to + // `@types/node`, while a package shipping its own declarations resolves to + // itself. Returning both over-approximates, the safe direction — whichever + // isn't installed drops out downstream. + assert.deepEqual(typeReferencePackageNames("node"), ["node", "@types/node"]); + // Scoped names mangle with a double underscore, TypeScript's convention. + assert.deepEqual(typeReferencePackageNames("@scope/pkg"), [ + "@scope/pkg", + "@types/scope__pkg", + ]); + assert.deepEqual(typeReferencePackageNames("./relative"), []); + assert.deepEqual(typeReferencePackageNames(""), []); +}); + test("importedPackageNames: prose in comments never becomes a package (Copilot, #1962)", () => { // The regex scan this replaced could not tell code from a comment, so // `// adapted from "react"` added `react` to the candidate set — and if that