Skip to content

Commit f6cb90a

Browse files
committed
Revert "Proposal: If there’s a package.json, only auto-import things in it, more or less (microsoft#31893)"
This reverts commit 60a1b1d.
1 parent e3ba3aa commit f6cb90a

18 files changed

+97
-751
lines changed

src/compiler/utilities.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -7485,7 +7485,7 @@ namespace ts {
74857485
export function getDirectoryPath(path: Path): Path;
74867486
/**
74877487
* Returns the path except for its basename. Semantics align with NodeJS's `path.dirname`
7488-
* except that we support URLs as well.
7488+
* except that we support URL's as well.
74897489
*
74907490
* ```ts
74917491
* getDirectoryPath("/path/to/file.ext") === "/path/to"

src/harness/fourslash.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -798,7 +798,7 @@ namespace FourSlash {
798798
const name = typeof include === "string" ? include : include.name;
799799
const found = nameToEntries.get(name);
800800
if (!found) throw this.raiseError(`No completion ${name} found`);
801-
assert(found.length === 1, `Must use 'exact' for multiple completions with same name: '${name}'`);
801+
assert(found.length === 1); // Must use 'exact' for multiple completions with same name
802802
this.verifyCompletionEntry(ts.first(found), include);
803803
}
804804
}

src/services/codefixes/importFixes.ts

+6-118
Original file line numberDiff line numberDiff line change
@@ -283,25 +283,13 @@ namespace ts.codefix {
283283
preferences: UserPreferences,
284284
): ReadonlyArray<FixAddNewImport | FixUseImportType> {
285285
const isJs = isSourceFileJS(sourceFile);
286-
const { allowsImporting } = createLazyPackageJsonDependencyReader(sourceFile, host);
287286
const choicesForEachExportingModule = flatMap(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) =>
288287
moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences, program.redirectTargetsMap)
289288
.map((moduleSpecifier): FixAddNewImport | FixUseImportType =>
290289
// `position` should only be undefined at a missing jsx namespace, in which case we shouldn't be looking for pure types.
291290
exportedSymbolIsTypeOnly && isJs ? { kind: ImportFixKind.ImportType, moduleSpecifier, position: Debug.assertDefined(position) } : { kind: ImportFixKind.AddNew, moduleSpecifier, importKind }));
292-
293-
// Sort by presence in package.json, then shortest paths first
294-
return sort(choicesForEachExportingModule, (a, b) => {
295-
const allowsImportingA = allowsImporting(a.moduleSpecifier);
296-
const allowsImportingB = allowsImporting(b.moduleSpecifier);
297-
if (allowsImportingA && !allowsImportingB) {
298-
return -1;
299-
}
300-
if (allowsImportingB && !allowsImportingA) {
301-
return 1;
302-
}
303-
return a.moduleSpecifier.length - b.moduleSpecifier.length;
304-
});
291+
// Sort to keep the shortest paths first
292+
return sort(choicesForEachExportingModule, (a, b) => a.moduleSpecifier.length - b.moduleSpecifier.length);
305293
}
306294

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

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

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

579-
export function forEachExternalModuleToImportFrom(checker: TypeChecker, host: LanguageServiceHost, preferences: UserPreferences, redirectTargetsMap: RedirectTargetsMap, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
580-
const { allowsImporting } = createLazyPackageJsonDependencyReader(from, host);
581-
const compilerOptions = host.getCompilationSettings();
582-
const getCanonicalFileName = hostGetCanonicalFileName(host);
564+
export function forEachExternalModuleToImportFrom(checker: TypeChecker, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
583565
forEachExternalModule(checker, allSourceFiles, (module, sourceFile) => {
584-
if (sourceFile === undefined && allowsImporting(stripQuotes(module.getName()))) {
566+
if (sourceFile === undefined || sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
585567
cb(module);
586568
}
587-
else if (sourceFile && sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
588-
const moduleSpecifier = getNodeModulesPackageNameFromFileName(sourceFile.fileName);
589-
if (!moduleSpecifier || allowsImporting(moduleSpecifier)) {
590-
cb(module);
591-
}
592-
}
593569
});
594-
595-
function getNodeModulesPackageNameFromFileName(importedFileName: string): string | undefined {
596-
const specifier = moduleSpecifiers.getModuleSpecifier(
597-
compilerOptions,
598-
from,
599-
toPath(from.fileName, /*basePath*/ undefined, getCanonicalFileName),
600-
importedFileName,
601-
host,
602-
allSourceFiles,
603-
preferences,
604-
redirectTargetsMap);
605-
606-
// Paths here are not node_modules, so we don’t care about them;
607-
// returning anything will trigger a lookup in package.json.
608-
if (!pathIsRelative(specifier) && !isRootedDiskPath(specifier)) {
609-
const components = getPathComponents(getPackageNameFromTypesPackageName(specifier)).slice(1);
610-
// Scoped packages
611-
if (startsWith(components[0], "@")) {
612-
return `${components[0]}/${components[1]}`;
613-
}
614-
return components[0];
615-
}
616-
}
617570
}
618571

619572
function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) {
@@ -667,69 +620,4 @@ namespace ts.codefix {
667620
// Need `|| "_"` to ensure result isn't empty.
668621
return !isStringANonContextualKeyword(res) ? res || "_" : `_${res}`;
669622
}
670-
671-
function createLazyPackageJsonDependencyReader(fromFile: SourceFile, host: LanguageServiceHost) {
672-
const packageJsonPaths = findPackageJsons(getDirectoryPath(fromFile.fileName), host);
673-
const dependencyIterator = readPackageJsonDependencies(host, packageJsonPaths);
674-
let seenDeps: Map<true> | undefined;
675-
let usesNodeCoreModules: boolean | undefined;
676-
return { allowsImporting };
677-
678-
function containsDependency(dependency: string) {
679-
if ((seenDeps || (seenDeps = createMap())).has(dependency)) {
680-
return true;
681-
}
682-
let packageName: string | void;
683-
while (packageName = dependencyIterator.next().value) {
684-
seenDeps.set(packageName, true);
685-
if (packageName === dependency) {
686-
return true;
687-
}
688-
}
689-
return false;
690-
}
691-
692-
function allowsImporting(moduleSpecifier: string): boolean {
693-
if (!packageJsonPaths.length) {
694-
return true;
695-
}
696-
697-
// If we’re in JavaScript, it can be difficult to tell whether the user wants to import
698-
// from Node core modules or not. We can start by seeing if the user is actually using
699-
// any node core modules, as opposed to simply having @types/node accidentally as a
700-
// dependency of a dependency.
701-
if (isSourceFileJS(fromFile) && JsTyping.nodeCoreModules.has(moduleSpecifier)) {
702-
if (usesNodeCoreModules === undefined) {
703-
usesNodeCoreModules = consumesNodeCoreModules(fromFile);
704-
}
705-
if (usesNodeCoreModules) {
706-
return true;
707-
}
708-
}
709-
710-
return containsDependency(moduleSpecifier)
711-
|| containsDependency(getTypesPackageName(moduleSpecifier));
712-
}
713-
}
714-
715-
function *readPackageJsonDependencies(host: LanguageServiceHost, packageJsonPaths: string[]) {
716-
type PackageJson = Record<typeof dependencyKeys[number], Record<string, string> | undefined>;
717-
const dependencyKeys = ["dependencies", "devDependencies", "optionalDependencies"] as const;
718-
for (const fileName of packageJsonPaths) {
719-
const content = readJson(fileName, { readFile: host.readFile ? host.readFile.bind(host) : sys.readFile }) as PackageJson;
720-
for (const key of dependencyKeys) {
721-
const dependencies = content[key];
722-
if (!dependencies) {
723-
continue;
724-
}
725-
for (const packageName in dependencies) {
726-
yield packageName;
727-
}
728-
}
729-
}
730-
}
731-
732-
function consumesNodeCoreModules(sourceFile: SourceFile): boolean {
733-
return some(sourceFile.imports, ({ text }) => JsTyping.nodeCoreModules.has(text));
734-
}
735623
}

0 commit comments

Comments
 (0)