Skip to content

(fix #50725, #50710) add file extensions in import statements #51702

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 7 commits into from
Dec 6, 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
6 changes: 3 additions & 3 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -880,9 +880,9 @@ function tryGetModuleNameFromRootDirs(rootDirs: readonly string[], moduleFileNam
return undefined;
}

return getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs
? removeExtensionAndIndexPostFix(shortest, ending, compilerOptions)
: removeFileExtension(shortest);
return getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.Classic
? removeFileExtension(shortest)
: removeExtensionAndIndexPostFix(shortest, ending, compilerOptions);
}

function tryGetModuleNameAsNodeModule({ path, isRedirect }: ModulePath, { getCanonicalFileName, sourceDirectory }: Info, importingSourceFile: SourceFile, host: ModuleSpecifierResolutionHost, options: CompilerOptions, userPreferences: UserPreferences, packageNameOnly?: boolean, overrideMode?: ResolutionMode): string | undefined {
Expand Down
63 changes: 44 additions & 19 deletions src/services/refactors/moveToNewFile.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getModuleSpecifier } from "../../compiler/moduleSpecifiers";
import {
AnyImportOrRequireStatement,
append,
Expand All @@ -16,13 +17,13 @@ import {
concatenate,
contains,
copyEntries,
createModuleSpecifierResolutionHost,
createTextRangeFromSpan,
Debug,
Declaration,
DeclarationStatement,
Diagnostics,
emptyArray,
ensurePathIsNonModuleName,
EnumDeclaration,
escapeLeadingUnderscores,
Expression,
Expand Down Expand Up @@ -102,9 +103,9 @@ import {
rangeContainsRange,
RefactorContext,
RefactorEditInfo,
removeFileExtension,
RequireOrImportCall,
RequireVariableStatement,
resolvePath,
ScriptTarget,
skipAlias,
some,
Expand Down Expand Up @@ -191,13 +192,12 @@ function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes

const currentDirectory = getDirectoryPath(oldFile.fileName);
const extension = extensionFromPath(oldFile.fileName);
const newModuleName = makeUniqueModuleName(getNewModuleName(usage.oldFileImportsFromNewFile, usage.movedSymbols), extension, currentDirectory, host);
const newFileNameWithExtension = newModuleName + extension;
const newFileBasename = makeUniqueModuleName(getNewModuleName(usage.oldFileImportsFromNewFile, usage.movedSymbols), extension, currentDirectory, host);

// If previous file was global, this is easy.
changes.createNewFile(oldFile, combinePaths(currentDirectory, newFileNameWithExtension), getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, newModuleName, preferences));
changes.createNewFile(oldFile, combinePaths(currentDirectory, newFileBasename + extension), getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, host, newFileBasename, extension, preferences));

addNewFileToTsconfig(program, changes, oldFile.fileName, newFileNameWithExtension, hostGetCanonicalFileName(host));
addNewFileToTsconfig(program, changes, oldFile.fileName, newFileBasename + extension, hostGetCanonicalFileName(host));
}

interface StatementRange {
Expand Down Expand Up @@ -258,7 +258,7 @@ function addNewFileToTsconfig(program: Program, changes: textChanges.ChangeTrack
}

function getNewStatementsAndRemoveFromOldFile(
oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, newModuleName: string, preferences: UserPreferences,
oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, host: LanguageServiceHost, newModuleName: string, extension: string, preferences: UserPreferences,
) {
const checker = program.getTypeChecker();
const prologueDirectives = takeWhile(oldFile.statements, isPrologueDirective);
Expand All @@ -269,16 +269,16 @@ function getNewStatementsAndRemoveFromOldFile(

const useEsModuleSyntax = !!oldFile.externalModuleIndicator;
const quotePreference = getQuotePreference(oldFile, preferences);
const importsFromNewFile = createOldFileImportsFromNewFile(usage.oldFileImportsFromNewFile, newModuleName, useEsModuleSyntax, quotePreference);
const importsFromNewFile = createOldFileImportsFromNewFile(oldFile, usage.oldFileImportsFromNewFile, newModuleName + extension, program, host, useEsModuleSyntax, quotePreference);
if (importsFromNewFile) {
insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true);
}

deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker);
deleteMovedStatements(oldFile, toMove.ranges, changes);
updateImportsInOtherFiles(changes, program, oldFile, usage.movedSymbols, newModuleName);
updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, newModuleName, extension);

const imports = getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByNewFile, usage.newFileImportsFromOldFile, changes, checker, useEsModuleSyntax, quotePreference);
const imports = getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByNewFile, usage.newFileImportsFromOldFile, changes, checker, program, host, useEsModuleSyntax, quotePreference);
const body = addExports(oldFile, toMove.all, usage.oldFileImportsFromNewFile, useEsModuleSyntax);
if (imports.length && body.length) {
return [
Expand Down Expand Up @@ -309,7 +309,9 @@ function deleteUnusedOldImports(oldFile: SourceFile, toMove: readonly Statement[
}
}

function updateImportsInOtherFiles(changes: textChanges.ChangeTracker, program: Program, oldFile: SourceFile, movedSymbols: ReadonlySymbolSet, newModuleName: string): void {
function updateImportsInOtherFiles(
changes: textChanges.ChangeTracker, program: Program, host: LanguageServiceHost, oldFile: SourceFile, movedSymbols: ReadonlySymbolSet, newModuleName: string, extension: string
): void {
const checker = program.getTypeChecker();
for (const sourceFile of program.getSourceFiles()) {
if (sourceFile === oldFile) continue;
Expand All @@ -324,7 +326,9 @@ function updateImportsInOtherFiles(changes: textChanges.ChangeTracker, program:
return !!symbol && movedSymbols.has(symbol);
};
deleteUnusedImports(sourceFile, importNode, changes, shouldMove); // These will be changed to imports from the new file
const newModuleSpecifier = combinePaths(getDirectoryPath(moduleSpecifierFromImport(importNode).text), newModuleName);

const pathToNewFileWithExtension = resolvePath(getDirectoryPath(oldFile.path), newModuleName + extension);
const newModuleSpecifier = getModuleSpecifier(program.getCompilerOptions(), sourceFile, sourceFile.path, pathToNewFileWithExtension, createModuleSpecifierResolutionHost(program, host));
const newImportDeclaration = filterImport(importNode, factory.createStringLiteral(newModuleSpecifier), shouldMove);
if (newImportDeclaration) changes.insertNodeAfter(sourceFile, statement, newImportDeclaration);

Expand Down Expand Up @@ -431,7 +435,15 @@ type SupportedImportStatement =
| ImportEqualsDeclaration
| VariableStatement;

function createOldFileImportsFromNewFile(newFileNeedExport: ReadonlySymbolSet, newFileNameWithExtension: string, useEs6Imports: boolean, quotePreference: QuotePreference): AnyImportOrRequireStatement | undefined {
function createOldFileImportsFromNewFile(
sourceFile: SourceFile,
newFileNeedExport: ReadonlySymbolSet,
newFileNameWithExtension: string,
program: Program,
host: LanguageServiceHost,
useEs6Imports: boolean,
quotePreference: QuotePreference
): AnyImportOrRequireStatement | undefined {
let defaultImport: Identifier | undefined;
const imports: string[] = [];
newFileNeedExport.forEach(symbol => {
Expand All @@ -442,20 +454,31 @@ function createOldFileImportsFromNewFile(newFileNeedExport: ReadonlySymbolSet, n
imports.push(symbol.name);
}
});
return makeImportOrRequire(defaultImport, imports, newFileNameWithExtension, useEs6Imports, quotePreference);
return makeImportOrRequire(sourceFile, defaultImport, imports, newFileNameWithExtension, program, host, useEs6Imports, quotePreference);
}

function makeImportOrRequire(defaultImport: Identifier | undefined, imports: readonly string[], path: string, useEs6Imports: boolean, quotePreference: QuotePreference): AnyImportOrRequireStatement | undefined {
path = ensurePathIsNonModuleName(path);
function makeImportOrRequire(
sourceFile: SourceFile,
defaultImport: Identifier | undefined,
imports: readonly string[],
newFileNameWithExtension: string,
program: Program,
host: LanguageServiceHost,
useEs6Imports: boolean,
quotePreference: QuotePreference
): AnyImportOrRequireStatement | undefined {
const pathToNewFile = resolvePath(getDirectoryPath(sourceFile.path), newFileNameWithExtension);
const pathToNewFileWithCorrectExtension = getModuleSpecifier(program.getCompilerOptions(), sourceFile, sourceFile.path, pathToNewFile, createModuleSpecifierResolutionHost(program, host));

if (useEs6Imports) {
const specifiers = imports.map(i => factory.createImportSpecifier(/*isTypeOnly*/ false, /*propertyName*/ undefined, factory.createIdentifier(i)));
return makeImportIfNecessary(defaultImport, specifiers, path, quotePreference);
return makeImportIfNecessary(defaultImport, specifiers, pathToNewFileWithCorrectExtension, quotePreference);
}
else {
Debug.assert(!defaultImport, "No default import should exist"); // If there's a default export, it should have been an es6 module.
const bindingElements = imports.map(i => factory.createBindingElement(/*dotDotDotToken*/ undefined, /*propertyName*/ undefined, i));
return bindingElements.length
? makeVariableStatement(factory.createObjectBindingPattern(bindingElements), /*type*/ undefined, createRequireCall(factory.createStringLiteral(path))) as RequireVariableStatement
? makeVariableStatement(factory.createObjectBindingPattern(bindingElements), /*type*/ undefined, createRequireCall(factory.createStringLiteral(pathToNewFileWithCorrectExtension))) as RequireVariableStatement
: undefined;
}
}
Expand Down Expand Up @@ -564,6 +587,8 @@ function getNewFileImportsAndAddExportInOldFile(
newFileImportsFromOldFile: ReadonlySymbolSet,
changes: textChanges.ChangeTracker,
checker: TypeChecker,
program: Program,
host: LanguageServiceHost,
useEsModuleSyntax: boolean,
quotePreference: QuotePreference,
): readonly SupportedImportStatement[] {
Expand Down Expand Up @@ -600,7 +625,7 @@ function getNewFileImportsAndAddExportInOldFile(
}
});

append(copiedOldImports, makeImportOrRequire(oldFileDefault, oldFileNamedImports, removeFileExtension(getBaseFileName(oldFile.fileName)), useEsModuleSyntax, quotePreference));
append(copiedOldImports, makeImportOrRequire(oldFile, oldFileDefault, oldFileNamedImports, getBaseFileName(oldFile.fileName), program, host, useEsModuleSyntax, quotePreference));
return copiedOldImports;
}

Expand Down
23 changes: 23 additions & 0 deletions tests/cases/fourslash/getEditsForFileRename_keepFileExtensions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/// <reference path="fourslash.ts"/>

// @Filename: /tsconfig.json
////{
//// "compilerOptions": {
//// "module": "Node16",
//// "rootDirs": ["src"]
//// }
////}

// @Filename: /src/person.ts
////export const name = 0;

// @Filename: /src/index.ts
////import {name} from "./person.js";

verify.getEditsForFileRename({
oldPath: '/src/person.ts',
newPath: '/src/vip.ts',
newFileContents: {
'/src/index.ts': 'import {name} from "./vip.js";',
},
});
70 changes: 70 additions & 0 deletions tests/cases/fourslash/moveToNewFile_importFileExtensions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/// <reference path="fourslash.ts"/>

// @Filename: /tsconfig.json
////{
//// "compilerOptions": {
//// "moduleResolution": "Node16",
//// }
////}

// @Filename: /package.json
//// { "type": "module" }

// @Filename: /src/main.ts
////[|export function someLibFn(): string {
//// return main();
////}|]
////
////function main(): string {
//// return "hello world!";
////}
////console.log(someLibFn());

// @Filename: /other.ts
////import { someLibFn } from "./src/main.js";
////
////function someOtherFn(): string {
//// return someLibFn();
////}

// @Filename: /act/action.ts
////import { someLibFn } from "../src/main.js";
////
////function doAction(): string {
//// return someLibFn();
////}


verify.moveToNewFile({
newFileContents: {
"/src/main.ts":
`import { someLibFn } from "./someLibFn.js";

export function main(): string {
return "hello world!";
}
console.log(someLibFn());`,

"/src/someLibFn.ts":
`import { main } from "./main.js";

export function someLibFn(): string {
return main();
}
`,

"/other.ts":
`import { someLibFn } from "./src/someLibFn.js";

function someOtherFn(): string {
return someLibFn();
}`,

"/act/action.ts":
`import { someLibFn } from "../src/someLibFn.js";

function doAction(): string {
return someLibFn();
}`,
}
});