Skip to content

Commit f1feb45

Browse files
committed
fix ATA toplevel dep detection
ATA tried to use the `_requiredBy` field to determine toplevel deps, but this is not portable. Not only is it unavailable in npm@>=7, but neither Yarn nor pnpm write this metadata to node_modules pkgjsons. Fixes: #44130
1 parent 2bbdb31 commit f1feb45

2 files changed

Lines changed: 50 additions & 11 deletions

File tree

src/jsTyping/jsTyping.ts

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ namespace ts.JsTyping {
99
}
1010

1111
interface PackageJson {
12-
_requiredBy?: string[];
1312
dependencies?: MapLike<string>;
1413
devDependencies?: MapLike<string>;
1514
name?: string;
@@ -264,6 +263,29 @@ namespace ts.JsTyping {
264263
return;
265264
}
266265

266+
// Collect all toplevel package names if we're looking in node_modules
267+
const parentPkgJsonPath = resolvePath(packagesFolderPath, "..", "package.json");
268+
const parentPkgJson: PackageJson = host.fileExists(parentPkgJsonPath) ? readConfigFile(parentPkgJsonPath, path => host.readFile(path)).config : undefined;
269+
const topLevelDeps = parentPkgJson && new Set();
270+
if (topLevelDeps) {
271+
const topLevelDependencies = getOwnKeys(parentPkgJson.dependencies || {});
272+
for (const dep of topLevelDependencies) {
273+
topLevelDeps.add(dep);
274+
}
275+
const topLevelDevDependencies = getOwnKeys(parentPkgJson.devDependencies || {});
276+
for (const dep of topLevelDevDependencies) {
277+
topLevelDeps.add(dep);
278+
}
279+
const topLevelOptionalDependencies = getOwnKeys(parentPkgJson.optionalDependencies || {});
280+
for (const dep of topLevelOptionalDependencies) {
281+
topLevelDeps.add(dep);
282+
}
283+
const topLevelPeerDependencies = getOwnKeys(parentPkgJson.peerDependencies || {});
284+
for (const dep of topLevelPeerDependencies) {
285+
topLevelDeps.add(dep);
286+
}
287+
}
288+
267289
// depth of 2, so we access `node_modules/foo` but not `node_modules/foo/bar`
268290
const fileNames = host.readDirectory(packagesFolderPath, [Extension.Json], /*excludes*/ undefined, /*includes*/ undefined, /*depth*/ 2);
269291
if (log) log(`Searching for typing names in ${packagesFolderPath}; all files: ${JSON.stringify(fileNames)}`);
@@ -277,19 +299,17 @@ namespace ts.JsTyping {
277299
const result = readConfigFile(normalizedFileName, (path: string) => host.readFile(path));
278300
const packageJson: PackageJson = result.config;
279301

280-
// npm 3's package.json contains a "_requiredBy" field
281-
// we should include all the top level module names for npm 2, and only module names whose
282-
// "_requiredBy" field starts with "#" or equals "/" for npm 3.
283-
if (baseFileName === "package.json" && packageJson._requiredBy &&
284-
filter(packageJson._requiredBy, (r: string) => r[0] === "#" || r === "/").length === 0) {
285-
continue;
286-
}
287-
288302
// If the package has its own d.ts typings, those will take precedence. Otherwise the package name will be used
289303
// to download d.ts files from DefinitelyTyped
290304
if (!packageJson.name) {
291305
continue;
292306
}
307+
308+
// This is the most portable way to check if a particular NPM package is, in fact, a toplevel dependency.
309+
if (baseFileName === "package.json" && topLevelDeps && !topLevelDeps.has(packageJson.name)) {
310+
continue;
311+
}
312+
293313
const ownTypes = packageJson.types || packageJson.typings;
294314
if (ownTypes) {
295315
const absolutePath = getNormalizedAbsolutePath(ownTypes, getDirectoryPath(normalizedFileName));

src/testRunner/unittests/tsserver/typingsInstaller.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -839,10 +839,29 @@ namespace ts.projectSystem {
839839
path: "/app.js",
840840
content: ""
841841
};
842+
const pkgJson = {
843+
path: "/package.json",
844+
content: JSON.stringify({
845+
dependencies: {
846+
jquery: "1.0.0"
847+
}
848+
})
849+
};
842850
const jsconfig = {
843851
path: "/jsconfig.json",
844852
content: JSON.stringify({})
845853
};
854+
// Should only accept direct dependencies.
855+
const commander = {
856+
path: "/node_modules/commander/index.js",
857+
content: ""
858+
};
859+
const commanderPackage = {
860+
path: "/node_modules/commander/package.json",
861+
content: JSON.stringify({
862+
name: "commander",
863+
})
864+
};
846865
const jquery = {
847866
path: "/node_modules/jquery/index.js",
848867
content: ""
@@ -860,10 +879,10 @@ namespace ts.projectSystem {
860879
path: "/tmp/node_modules/@types/jquery/index.d.ts",
861880
content: ""
862881
};
863-
const host = createServerHost([app, jsconfig, jquery, jqueryPackage, nestedPackage]);
882+
const host = createServerHost([app, jsconfig, pkgJson, commander, commanderPackage, jquery, jqueryPackage, nestedPackage]);
864883
const installer = new (class extends Installer {
865884
constructor() {
866-
super(host, { globalTypingsCacheLocation: "/tmp", typesRegistry: createTypesRegistry("jquery", "nested") });
885+
super(host, { globalTypingsCacheLocation: "/tmp", typesRegistry: createTypesRegistry("jquery", "nested", "commander") });
867886
}
868887
installWorker(_requestId: number, args: string[], _cwd: string, cb: TI.RequestCompletedAction) {
869888
assert.deepEqual(args, [`@types/jquery@ts${versionMajorMinor}`]);

0 commit comments

Comments
 (0)