Skip to content

Commit f50c15a

Browse files
zkatzkat
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. This also adds support for ATA acquiring types for scoped packages. Fixes: #44130
1 parent 2bbdb31 commit f50c15a

File tree

2 files changed

+210
-74
lines changed

2 files changed

+210
-74
lines changed

‎src/jsTyping/jsTyping.ts

+96-71
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;
@@ -152,17 +151,8 @@ namespace ts.JsTyping {
152151
const possibleSearchDirs = new Set(fileNames.map(getDirectoryPath));
153152
possibleSearchDirs.add(projectRootPath);
154153
possibleSearchDirs.forEach((searchDir) => {
155-
const packageJsonPath = combinePaths(searchDir, "package.json");
156-
getTypingNamesFromJson(packageJsonPath, filesToWatch);
157-
158-
const bowerJsonPath = combinePaths(searchDir, "bower.json");
159-
getTypingNamesFromJson(bowerJsonPath, filesToWatch);
160-
161-
const bowerComponentsPath = combinePaths(searchDir, "bower_components");
162-
getTypingNamesFromPackagesFolder(bowerComponentsPath, filesToWatch);
163-
164-
const nodeModulesPath = combinePaths(searchDir, "node_modules");
165-
getTypingNamesFromPackagesFolder(nodeModulesPath, filesToWatch);
154+
getTypingNames(searchDir, "bower.json", "bower_components", filesToWatch);
155+
getTypingNames(searchDir, "package.json", "node_modules", filesToWatch);
166156
});
167157
if(!typeAcquisition.disableFilenameBasedTypeAcquisition) {
168158
getTypingNamesFromSourceFileNames(fileNames);
@@ -214,17 +204,104 @@ namespace ts.JsTyping {
214204
}
215205

216206
/**
217-
* Get the typing info from common package manager json files like package.json or bower.json
207+
* Adds inferred typings from manifest/module pairs (think package.json + node_modules)
208+
*
209+
* @param projectRootPath is the path to the directory where to look for package.json, bower.json and other typing information
210+
* @param manifestName is the name of the manifest (package.json or bower.json)
211+
* @param modulesDirName is the directory name for modules (node_modules or bower_components). Should be lowercase!
212+
* @param filesToWatch are the files to watch for changes. We will push things into this array.
218213
*/
219-
function getTypingNamesFromJson(jsonPath: string, filesToWatch: Push<string>) {
220-
if (!host.fileExists(jsonPath)) {
214+
function getTypingNames(projectRootPath: string, manifestName: string, modulesDirName: string, filesToWatch: string[]): void {
215+
// First, we check the manifests themselves. They're not
216+
// _required_, but they allow us to do some filtering when dealing
217+
// with big flat dep directories.
218+
const manifestPath = combinePaths(projectRootPath, manifestName);
219+
let manifest;
220+
let manifestTypingNames;
221+
if (host.fileExists(manifestPath)) {
222+
filesToWatch.push(manifestPath);
223+
manifest = readConfigFile(manifestPath, path => host.readFile(path)).config;
224+
manifestTypingNames = flatMap([manifest.dependencies, manifest.devDependencies, manifest.optionalDependencies, manifest.peerDependencies], getOwnKeys);
225+
addInferredTypings(manifestTypingNames, `Typing names in '${manifestPath}' dependencies`);
226+
}
227+
228+
// Now we scan the directories for typing information in
229+
// already-installed dependencies (if present). Note that this
230+
// step happens regardless of whether a manifest was present,
231+
// which is certainly a valid configuration, if an unusual one.
232+
const packagesFolderPath = combinePaths(projectRootPath, modulesDirName);
233+
filesToWatch.push(packagesFolderPath);
234+
if (!host.directoryExists(packagesFolderPath)) {
221235
return;
222236
}
223237

224-
filesToWatch.push(jsonPath);
225-
const jsonConfig: PackageJson = readConfigFile(jsonPath, path => host.readFile(path)).config;
226-
const jsonTypingNames = flatMap([jsonConfig.dependencies, jsonConfig.devDependencies, jsonConfig.optionalDependencies, jsonConfig.peerDependencies], getOwnKeys);
227-
addInferredTypings(jsonTypingNames, `Typing names in '${jsonPath}' dependencies`);
238+
// There's two cases we have to take into account here:
239+
// 1. If manifest is undefined, then we're not using a manifest.
240+
// That means that we should scan _all_ dependencies at the top
241+
// level of the modulesDir.
242+
// 2. If manifest is defined, then we can do some special
243+
// filtering to reduce the amount of scanning we need to do.
244+
//
245+
// Previous versions of this algorithm checked for a `_requiredBy`
246+
// field in the package.json, but that field is only present in
247+
// `npm@>=3 <7`.
248+
249+
// Package names that do **not** provide their own typings, so
250+
// we'll look them up.
251+
const packageNames: string[] = [];
252+
253+
const dependencyManifestNames = manifestTypingNames
254+
// This is #1 described above.
255+
? manifestTypingNames.map(typingName => combinePaths(packagesFolderPath, typingName, manifestName))
256+
// And #2. Depth = 3 because scoped packages look like `node_modules/@foo/bar/package.json`
257+
: host.readDirectory(packagesFolderPath, [Extension.Json], /*excludes*/ undefined, /*includes*/ undefined, /*depth*/ 3)
258+
.filter(manifestPath => {
259+
if (getBaseFileName(manifestPath) !== manifestName) {
260+
return false;
261+
}
262+
// It's ok to treat
263+
// `node_modules/@foo/bar/package.json` as a manifest,
264+
// but not `node_modules/jquery/nested/package.json`.
265+
// We only assume depth 3 is ok for formally scoped
266+
// packages. So that needs this dance here.
267+
const pathComponents = getPathComponents(normalizePath(manifestPath));
268+
const isScoped = pathComponents[pathComponents.length - 3][0] === "@";
269+
return isScoped && pathComponents[pathComponents.length - 4].toLowerCase() === modulesDirName || // `node_modules/@foo/bar`
270+
!isScoped && pathComponents[pathComponents.length - 3].toLowerCase() === modulesDirName; // `node_modules/foo`
271+
});
272+
273+
if (log) log(`Searching for typing names in ${packagesFolderPath}; all files: ${JSON.stringify(dependencyManifestNames)}`);
274+
275+
// Once we have the names of things to look up, we iterate over
276+
// and either collect their included typings, or add them to the
277+
// list of typings we need to look up separately.
278+
for (const manifestPath of dependencyManifestNames) {
279+
const normalizedFileName = normalizePath(manifestPath);
280+
const result = readConfigFile(normalizedFileName, (path: string) => host.readFile(path));
281+
const manifest: PackageJson = result.config;
282+
283+
// If the package has its own d.ts typings, those will take precedence. Otherwise the package name will be used
284+
// to download d.ts files from DefinitelyTyped
285+
if (!manifest.name) {
286+
continue;
287+
}
288+
const ownTypes = manifest.types || manifest.typings;
289+
if (ownTypes) {
290+
const absolutePath = getNormalizedAbsolutePath(ownTypes, getDirectoryPath(normalizedFileName));
291+
if (host.fileExists(absolutePath)) {
292+
if (log) log(` Package '${manifest.name}' provides its own types.`);
293+
inferredTypings.set(manifest.name, absolutePath);
294+
}
295+
else {
296+
if (log) log(` Package '${manifest.name}' provides its own types but they are missing.`);
297+
}
298+
}
299+
else {
300+
packageNames.push(manifest.name);
301+
}
302+
}
303+
304+
addInferredTypings(packageNames, " Found package names");
228305
}
229306

230307
/**
@@ -251,58 +328,6 @@ namespace ts.JsTyping {
251328
addInferredTyping("react");
252329
}
253330
}
254-
255-
/**
256-
* Infer typing names from packages folder (ex: node_module, bower_components)
257-
* @param packagesFolderPath is the path to the packages folder
258-
*/
259-
function getTypingNamesFromPackagesFolder(packagesFolderPath: string, filesToWatch: Push<string>) {
260-
filesToWatch.push(packagesFolderPath);
261-
262-
// Todo: add support for ModuleResolutionHost too
263-
if (!host.directoryExists(packagesFolderPath)) {
264-
return;
265-
}
266-
267-
// depth of 2, so we access `node_modules/foo` but not `node_modules/foo/bar`
268-
const fileNames = host.readDirectory(packagesFolderPath, [Extension.Json], /*excludes*/ undefined, /*includes*/ undefined, /*depth*/ 2);
269-
if (log) log(`Searching for typing names in ${packagesFolderPath}; all files: ${JSON.stringify(fileNames)}`);
270-
const packageNames: string[] = [];
271-
for (const fileName of fileNames) {
272-
const normalizedFileName = normalizePath(fileName);
273-
const baseFileName = getBaseFileName(normalizedFileName);
274-
if (baseFileName !== "package.json" && baseFileName !== "bower.json") {
275-
continue;
276-
}
277-
const result = readConfigFile(normalizedFileName, (path: string) => host.readFile(path));
278-
const packageJson: PackageJson = result.config;
279-
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-
288-
// If the package has its own d.ts typings, those will take precedence. Otherwise the package name will be used
289-
// to download d.ts files from DefinitelyTyped
290-
if (!packageJson.name) {
291-
continue;
292-
}
293-
const ownTypes = packageJson.types || packageJson.typings;
294-
if (ownTypes) {
295-
const absolutePath = getNormalizedAbsolutePath(ownTypes, getDirectoryPath(normalizedFileName));
296-
if (log) log(` Package '${packageJson.name}' provides its own types.`);
297-
inferredTypings.set(packageJson.name, absolutePath);
298-
}
299-
else {
300-
packageNames.push(packageJson.name);
301-
}
302-
}
303-
addInferredTypings(packageNames, " Found package names");
304-
}
305-
306331
}
307332

308333
export const enum NameValidationResult {

‎src/testRunner/unittests/tsserver/typingsInstaller.ts

+114-3
Original file line numberDiff line numberDiff line change
@@ -834,15 +834,101 @@ namespace ts.projectSystem {
834834
checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path]);
835835
});
836836

837+
it("configured scoped name projects discover from node_modules", () => {
838+
const app = {
839+
path: "/app.js",
840+
content: ""
841+
};
842+
const pkgJson = {
843+
path: "/package.json",
844+
content: JSON.stringify({
845+
dependencies: {
846+
"@zkat/cacache": "1.0.0"
847+
}
848+
})
849+
};
850+
const jsconfig = {
851+
path: "/jsconfig.json",
852+
content: JSON.stringify({})
853+
};
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+
};
865+
const cacache = {
866+
path: "/node_modules/@zkat/cacache/index.js",
867+
content: ""
868+
};
869+
const cacachePackage = {
870+
path: "/node_modules/@zkat/cacache/package.json",
871+
content: JSON.stringify({ name: "@zkat/cacache" })
872+
};
873+
const cacacheDTS = {
874+
path: "/tmp/node_modules/@types/zkat__cacache/index.d.ts",
875+
content: ""
876+
};
877+
const host = createServerHost([app, jsconfig, pkgJson, commander, commanderPackage, cacache, cacachePackage]);
878+
const installer = new (class extends Installer {
879+
constructor() {
880+
super(host, { globalTypingsCacheLocation: "/tmp", typesRegistry: createTypesRegistry("zkat__cacache", "nested", "commander") });
881+
}
882+
installWorker(_requestId: number, args: string[], _cwd: string, cb: TI.RequestCompletedAction) {
883+
assert.deepEqual(args, [`@types/zkat__cacache@ts${versionMajorMinor}`]);
884+
const installedTypings = ["@types/zkat__cacache"];
885+
const typingFiles = [cacacheDTS];
886+
executeCommand(this, host, installedTypings, typingFiles, cb);
887+
}
888+
})();
889+
890+
const projectService = createProjectService(host, { useSingleInferredProject: true, typingsInstaller: installer });
891+
projectService.openClientFile(app.path);
892+
893+
checkNumberOfProjects(projectService, { configuredProjects: 1 });
894+
const p = configuredProjectAt(projectService, 0);
895+
checkProjectActualFiles(p, [app.path, jsconfig.path]);
896+
897+
installer.installAll(/*expectedCount*/ 1);
898+
899+
checkNumberOfProjects(projectService, { configuredProjects: 1 });
900+
host.checkTimeoutQueueLengthAndRun(2);
901+
checkProjectActualFiles(p, [app.path, cacacheDTS.path, jsconfig.path]);
902+
});
903+
837904
it("configured projects discover from node_modules", () => {
838905
const app = {
839906
path: "/app.js",
840907
content: ""
841908
};
909+
const pkgJson = {
910+
path: "/package.json",
911+
content: JSON.stringify({
912+
dependencies: {
913+
jquery: "1.0.0"
914+
}
915+
})
916+
};
842917
const jsconfig = {
843918
path: "/jsconfig.json",
844919
content: JSON.stringify({})
845920
};
921+
// Should only accept direct dependencies.
922+
const commander = {
923+
path: "/node_modules/commander/index.js",
924+
content: ""
925+
};
926+
const commanderPackage = {
927+
path: "/node_modules/commander/package.json",
928+
content: JSON.stringify({
929+
name: "commander",
930+
})
931+
};
846932
const jquery = {
847933
path: "/node_modules/jquery/index.js",
848934
content: ""
@@ -860,10 +946,10 @@ namespace ts.projectSystem {
860946
path: "/tmp/node_modules/@types/jquery/index.d.ts",
861947
content: ""
862948
};
863-
const host = createServerHost([app, jsconfig, jquery, jqueryPackage, nestedPackage]);
949+
const host = createServerHost([app, jsconfig, pkgJson, commander, commanderPackage, jquery, jqueryPackage, nestedPackage]);
864950
const installer = new (class extends Installer {
865951
constructor() {
866-
super(host, { globalTypingsCacheLocation: "/tmp", typesRegistry: createTypesRegistry("jquery", "nested") });
952+
super(host, { globalTypingsCacheLocation: "/tmp", typesRegistry: createTypesRegistry("jquery", "nested", "commander") });
867953
}
868954
installWorker(_requestId: number, args: string[], _cwd: string, cb: TI.RequestCompletedAction) {
869955
assert.deepEqual(args, [`@types/jquery@ts${versionMajorMinor}`]);
@@ -901,7 +987,7 @@ namespace ts.projectSystem {
901987
content: ""
902988
};
903989
const jqueryPackage = {
904-
path: "/bower_components/jquery/package.json",
990+
path: "/bower_components/jquery/bower.json",
905991
content: JSON.stringify({ name: "jquery" })
906992
};
907993
const jqueryDTS = {
@@ -1556,6 +1642,31 @@ namespace ts.projectSystem {
15561642
});
15571643
});
15581644

1645+
it("should support scoped packages", () => {
1646+
const app = {
1647+
path: "/app.js",
1648+
content: "",
1649+
};
1650+
const a = {
1651+
path: "/node_modules/@a/b/package.json",
1652+
content: JSON.stringify({ name: "@a/b" }),
1653+
};
1654+
const host = createServerHost([app, a]);
1655+
const cache = new Map<string, JsTyping.CachedTyping>();
1656+
const logger = trackingLogger();
1657+
const result = JsTyping.discoverTypings(host, logger.log, [app.path], getDirectoryPath(app.path as Path), emptySafeList, cache, { enable: true }, /*unresolvedImports*/ [], emptyMap);
1658+
assert.deepEqual(logger.finish(), [
1659+
'Searching for typing names in /node_modules; all files: ["/node_modules/@a/b/package.json"]',
1660+
' Found package names: ["@a/b"]',
1661+
"Inferred typings from unresolved imports: []",
1662+
'Result: {"cachedTypingPaths":[],"newTypingNames":["@a/b"],"filesToWatch":["/bower_components","/node_modules"]}',
1663+
]);
1664+
assert.deepEqual(result, {
1665+
cachedTypingPaths: [],
1666+
newTypingNames: ["@a/b"],
1667+
filesToWatch: ["/bower_components", "/node_modules"],
1668+
});
1669+
});
15591670
it("should install expired typings", () => {
15601671
const app = {
15611672
path: "/a/app.js",

0 commit comments

Comments
 (0)