Skip to content
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.

Sign up for GitHub

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

Revert "Proposal: If there’s a package.json, only auto-import things … #32444

Closed
wants to merge 1 commit into from
Closed

Revert "Proposal: If there’s a package.json, only auto-import things … #32444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7485,7 +7485,7 @@ namespace ts {
export function getDirectoryPath(path: Path): Path;
/**
* Returns the path except for its basename. Semantics align with NodeJS's `path.dirname`
* except that we support URLs as well.
* except that we support URL's as well.
*
* ```ts
* getDirectoryPath("/path/to/file.ext") === "/path/to"
Expand Down
2 changes: 1 addition & 1 deletion src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ namespace FourSlash {
const name = typeof include === "string" ? include : include.name;
const found = nameToEntries.get(name);
if (!found) throw this.raiseError(`No completion ${name} found`);
assert(found.length === 1, `Must use 'exact' for multiple completions with same name: '${name}'`);
assert(found.length === 1); // Must use 'exact' for multiple completions with same name
this.verifyCompletionEntry(ts.first(found), include);
}
}
Expand Down
124 changes: 6 additions & 118 deletions src/services/codefixes/importFixes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,25 +283,13 @@ namespace ts.codefix {
preferences: UserPreferences,
): ReadonlyArray<FixAddNewImport | FixUseImportType> {
const isJs = isSourceFileJS(sourceFile);
const { allowsImporting } = createLazyPackageJsonDependencyReader(sourceFile, host);
const choicesForEachExportingModule = flatMap(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) =>
moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences, program.redirectTargetsMap)
.map((moduleSpecifier): FixAddNewImport | FixUseImportType =>
// `position` should only be undefined at a missing jsx namespace, in which case we shouldn't be looking for pure types.
exportedSymbolIsTypeOnly && isJs ? { kind: ImportFixKind.ImportType, moduleSpecifier, position: Debug.assertDefined(position) } : { kind: ImportFixKind.AddNew, moduleSpecifier, importKind }));

// Sort by presence in package.json, then shortest paths first
return sort(choicesForEachExportingModule, (a, b) => {
const allowsImportingA = allowsImporting(a.moduleSpecifier);
const allowsImportingB = allowsImporting(b.moduleSpecifier);
if (allowsImportingA && !allowsImportingB) {
return -1;
}
if (allowsImportingB && !allowsImportingA) {
return 1;
}
return a.moduleSpecifier.length - b.moduleSpecifier.length;
});
// Sort to keep the shortest paths first
return sort(choicesForEachExportingModule, (a, b) => a.moduleSpecifier.length - b.moduleSpecifier.length);
}

function getFixesForAddImport(
Expand Down Expand Up @@ -392,8 +380,7 @@ namespace ts.codefix {
// "default" is a keyword and not a legal identifier for the import, so we don't expect it here
Debug.assert(symbolName !== InternalSymbolName.Default);

const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, checker, program, preferences, host);
const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) =>
const fixes = arrayFrom(flatMapIterator(getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, checker, program).entries(), ([_, exportInfos]) =>
getFixForImport(exportInfos, symbolName, symbolToken.getStart(sourceFile), program, sourceFile, host, preferences)));
return { fixes, symbolName };
}
Expand All @@ -406,16 +393,14 @@ namespace ts.codefix {
sourceFile: SourceFile,
checker: TypeChecker,
program: Program,
preferences: UserPreferences,
host: LanguageServiceHost
): ReadonlyMap<ReadonlyArray<SymbolExportInfo>> {
// For each original symbol, keep all re-exports of that symbol together so we can call `getCodeActionsForImport` on the whole group at once.
// Maps symbol id to info for modules providing that symbol (original export + re-exports).
const originalSymbolToExportInfos = createMultiMap<SymbolExportInfo>();
function addSymbol(moduleSymbol: Symbol, exportedSymbol: Symbol, importKind: ImportKind): void {
originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { moduleSymbol, importKind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exportedSymbol, checker) });
}
forEachExternalModuleToImportFrom(checker, host, preferences, program.redirectTargetsMap, sourceFile, program.getSourceFiles(), moduleSymbol => {
forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => {
cancellationToken.throwIfCancellationRequested();

const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions());
Expand Down Expand Up @@ -576,44 +561,12 @@ namespace ts.codefix {
return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning));
}

export function forEachExternalModuleToImportFrom(checker: TypeChecker, host: LanguageServiceHost, preferences: UserPreferences, redirectTargetsMap: RedirectTargetsMap, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
const { allowsImporting } = createLazyPackageJsonDependencyReader(from, host);
const compilerOptions = host.getCompilationSettings();
const getCanonicalFileName = hostGetCanonicalFileName(host);
export function forEachExternalModuleToImportFrom(checker: TypeChecker, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
forEachExternalModule(checker, allSourceFiles, (module, sourceFile) => {
if (sourceFile === undefined && allowsImporting(stripQuotes(module.getName()))) {
if (sourceFile === undefined || sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
cb(module);
}
else if (sourceFile && sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
const moduleSpecifier = getNodeModulesPackageNameFromFileName(sourceFile.fileName);
if (!moduleSpecifier || allowsImporting(moduleSpecifier)) {
cb(module);
}
}
});

function getNodeModulesPackageNameFromFileName(importedFileName: string): string | undefined {
const specifier = moduleSpecifiers.getModuleSpecifier(
compilerOptions,
from,
toPath(from.fileName, /*basePath*/ undefined, getCanonicalFileName),
importedFileName,
host,
allSourceFiles,
preferences,
redirectTargetsMap);

// Paths here are not node_modules, so we don’t care about them;
// returning anything will trigger a lookup in package.json.
if (!pathIsRelative(specifier) && !isRootedDiskPath(specifier)) {
const components = getPathComponents(getPackageNameFromTypesPackageName(specifier)).slice(1);
// Scoped packages
if (startsWith(components[0], "@")) {
return `${components[0]}/${components[1]}`;
}
return components[0];
}
}
}

function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) {
Expand Down Expand Up @@ -667,69 +620,4 @@ namespace ts.codefix {
// Need `|| "_"` to ensure result isn't empty.
return !isStringANonContextualKeyword(res) ? res || "_" : `_${res}`;
}

function createLazyPackageJsonDependencyReader(fromFile: SourceFile, host: LanguageServiceHost) {
const packageJsonPaths = findPackageJsons(getDirectoryPath(fromFile.fileName), host);
const dependencyIterator = readPackageJsonDependencies(host, packageJsonPaths);
let seenDeps: Map<true> | undefined;
let usesNodeCoreModules: boolean | undefined;
return { allowsImporting };

function containsDependency(dependency: string) {
if ((seenDeps || (seenDeps = createMap())).has(dependency)) {
return true;
}
let packageName: string | void;
while (packageName = dependencyIterator.next().value) {
seenDeps.set(packageName, true);
if (packageName === dependency) {
return true;
}
}
return false;
}

function allowsImporting(moduleSpecifier: string): boolean {
if (!packageJsonPaths.length) {
return true;
}

// If we’re in JavaScript, it can be difficult to tell whether the user wants to import
// from Node core modules or not. We can start by seeing if the user is actually using
// any node core modules, as opposed to simply having @types/node accidentally as a
// dependency of a dependency.
if (isSourceFileJS(fromFile) && JsTyping.nodeCoreModules.has(moduleSpecifier)) {
if (usesNodeCoreModules === undefined) {
usesNodeCoreModules = consumesNodeCoreModules(fromFile);
}
if (usesNodeCoreModules) {
return true;
}
}

return containsDependency(moduleSpecifier)
|| containsDependency(getTypesPackageName(moduleSpecifier));
}
}

function *readPackageJsonDependencies(host: LanguageServiceHost, packageJsonPaths: string[]) {
type PackageJson = Record<typeof dependencyKeys[number], Record<string, string> | undefined>;
const dependencyKeys = ["dependencies", "devDependencies", "optionalDependencies"] as const;
for (const fileName of packageJsonPaths) {
const content = readJson(fileName, { readFile: host.readFile ? host.readFile.bind(host) : sys.readFile }) as PackageJson;
for (const key of dependencyKeys) {
const dependencies = content[key];
if (!dependencies) {
continue;
}
for (const packageName in dependencies) {
yield packageName;
}
}
}
}

function consumesNodeCoreModules(sourceFile: SourceFile): boolean {
return some(sourceFile.imports, ({ text }) => JsTyping.nodeCoreModules.has(text));
}
}
Loading