Skip to content

Enable path completions for node12/nodenext #47836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 68 additions & 4 deletions src/services/stringCompletions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,20 @@ namespace ts.Completions.StringCompletions {
}
}

function isEmitResolutionKindUsingNodeModules(compilerOptions: CompilerOptions): boolean {
return getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs ||
getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.Node12 ||
getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeNext;
}

function isEmitModuleResolutionRespectingExportMaps(compilerOptions: CompilerOptions) {
return getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.Node12 ||
getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeNext;
}

function getSupportedExtensionsForModuleResolution(compilerOptions: CompilerOptions): readonly Extension[][] {
const extensions = getSupportedExtensions(compilerOptions);
return getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs ?
return isEmitResolutionKindUsingNodeModules(compilerOptions) ?
getSupportedExtensionsWithJsonIfResolveJsonModule(compilerOptions, extensions) :
extensions;
}
Expand Down Expand Up @@ -549,7 +560,7 @@ namespace ts.Completions.StringCompletions {

getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, fragmentDirectory, extensionOptions, result);

if (getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs) {
if (isEmitResolutionKindUsingNodeModules(compilerOptions)) {
// If looking for a global package name, don't just include everything in `node_modules` because that includes dependencies' own dependencies.
// (But do if we didn't find anything, e.g. 'package.json' missing.)
let foundGlobal = false;
Expand All @@ -562,12 +573,65 @@ namespace ts.Completions.StringCompletions {
}
}
if (!foundGlobal) {
forEachAncestorDirectory(scriptPath, ancestor => {
let ancestorLookup: (directory: string) => void | undefined = ancestor => {
const nodeModules = combinePaths(ancestor, "node_modules");
if (tryDirectoryExists(host, nodeModules)) {
getCompletionEntriesForDirectoryFragment(fragment, nodeModules, extensionOptions, host, /*exclude*/ undefined, result);
}
});
};
if (fragmentDirectory && isEmitModuleResolutionRespectingExportMaps(compilerOptions)) {
const nodeModulesDirectoryLookup = ancestorLookup;
ancestorLookup = ancestor => {
const components = getPathComponents(fragment);
components.shift(); // shift off empty root
let packagePath = components.shift();
if (!packagePath) {
return nodeModulesDirectoryLookup(ancestor);
}
if (startsWith(packagePath, "@")) {
const subName = components.shift();
if (!subName) {
return nodeModulesDirectoryLookup(ancestor);
}
packagePath = combinePaths(packagePath, subName);
}
const packageFile = combinePaths(ancestor, "node_modules", packagePath, "package.json");
if (tryFileExists(host, packageFile)) {
const packageJson = readJson(packageFile, host as { readFile: (filename: string) => string | undefined });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you know this function creates a SourceFile with parse diagnostics?? 😵

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I buy it - our json document parsing is more lenient than the JSON.parse one because we allow comments and trailing commas, so I can understand preferring to use it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm crashes on comments and trailing commas. My strong suspicion is that readJson leaked into package.json-reading code only because it nicely avoids the need for a try/catch and it’s totally non-obvious that it’s doing so much extra work. I’ve been meaning to rename it and use a wrapped JSON.parse on package.jsons. It might make a little bit of a perf difference in module resolution when you have a lot of node_modules.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably true; but I mean, it's probably most important to just use the cached package.json results where possible first 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, definitely.

const exports = (packageJson as any).exports;
if (exports) {
if (typeof exports !== "object" || exports === null) { // eslint-disable-line no-null/no-null
return; // null exports or entrypoint only, no sub-modules available
}
const keys = getOwnKeys(exports);
const fragmentSubpath = components.join("/");
const processedKeys = mapDefined(keys, k => {
if (k === ".") return undefined;
if (!startsWith(k, "./")) return undefined;
const subpath = k.substring(2);
if (!startsWith(subpath, fragmentSubpath)) return undefined;
// subpath is a valid export (barring conditions, which we don't currently check here)
if (!stringContains(subpath, "*")) {
return subpath;
}
// pattern export - only return everything up to the `*`, so the user can autocomplete, then
// keep filling in the pattern (we could speculatively return a list of options by hitting disk,
// but conditions will make that somewhat awkward, as each condition may have a different set of possible
// options for the `*`.
return subpath.slice(0, subpath.indexOf("*"));
});
forEach(processedKeys, k => {
if (k) {
result.push(nameAndKind(k, ScriptElementKind.externalModuleName, /*extension*/ undefined));
}
});
return;
}
}
return nodeModulesDirectoryLookup(ancestor);
};
}
forEachAncestorDirectory(scriptPath, ancestorLookup);
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/baselines/reference/nodeNextPathCompletions.baseline
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
[
{
"marker": {
"fileName": "/src/foo.ts",
"position": 30,
"name": ""
},
"completionList": {
"isGlobalCompletion": false,
"isMemberCompletion": false,
"isNewIdentifierLocation": true,
"entries": [
{
"name": "dependency",
"kind": "external module name",
"kindModifiers": "",
"sortText": "11",
"displayParts": [
{
"text": "dependency",
"kind": "text"
}
],
"tags": []
}
]
}
}
]
43 changes: 43 additions & 0 deletions tests/cases/fourslash/server/nodeNextPathCompletions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/// <reference path="../fourslash.ts" />

// @Filename: /node_modules/dependency/package.json
//// {
//// "type": "module",
//// "name": "dependency",
//// "version": "1.0.0",
//// "exports": {
//// ".": {
//// "types": "./lib/index.d.ts"
//// },
//// "./lol": {
//// "types": "./lib/lol.d.ts"
//// },
//// "./dir/*": "./lib/*"
//// }
//// }

// @Filename: /node_modules/dependency/lib/index.d.ts
//// export function fooFromIndex(): void;

// @Filename: /node_modules/dependency/lib/lol.d.ts
//// export function fooFromLol(): void;

// @Filename: /package.json
//// {
//// "type": "module",
//// "dependencies": {
//// "dependency": "^1.0.0"
//// }
//// }

// @Filename: /tsconfig.json
//// { "compilerOptions": { "module": "nodenext" }, "files": ["./src/foo.ts"] }

// @Filename: /src/foo.ts
//// import { fooFromIndex } from "/**/";

verify.baselineCompletions();
edit.insert("dependency/");
verify.completions({ exact: ["lol", "dir/"], isNewIdentifierLocation: true });
edit.insert("l");
verify.completions({ exact: ["lol"], isNewIdentifierLocation: true });