Skip to content

Commit 1586cee

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: microsoft#44130
1 parent 4761ba6 commit 1586cee

File tree

2 files changed

+50
-10
lines changed

2 files changed

+50
-10
lines changed

src/jsTyping/jsTyping.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,29 @@ namespace ts.JsTyping {
264264
return;
265265
}
266266

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

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-
288303
// If the package has its own d.ts typings, those will take precedence. Otherwise the package name will be used
289304
// to download d.ts files from DefinitelyTyped
290305
if (!packageJson.name) {
291306
continue;
292307
}
308+
309+
// This is the most portable way to check if a particular NPM package is, in fact, a toplevel dependency.
310+
if (baseFileName === "package.json" && topLevelDeps && !topLevelDeps.has(packageJson.name)) {
311+
continue;
312+
}
313+
293314
const ownTypes = packageJson.types || packageJson.typings;
294315
if (ownTypes) {
295316
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)