Skip to content

Commit c84838b

Browse files
authored
Avoid rewriting bare module specifiers on rename when fix is not verifiably correct (microsoft#41959)
1 parent 1e4a5c9 commit c84838b

File tree

3 files changed

+49
-5
lines changed

3 files changed

+49
-5
lines changed

src/services/getEditsForFileRename.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ namespace ts {
151151
const toImport = oldFromNew !== undefined
152152
// If we're at the new location (file was already renamed), need to redo module resolution starting from the old location.
153153
// TODO:GH#18217
154-
? getSourceFileToImportFromResolved(resolveModuleName(importLiteral.text, oldImportFromPath, program.getCompilerOptions(), host as ModuleResolutionHost),
154+
? getSourceFileToImportFromResolved(importLiteral, resolveModuleName(importLiteral.text, oldImportFromPath, program.getCompilerOptions(), host as ModuleResolutionHost),
155155
oldToNew, allFiles)
156156
: getSourceFileToImport(importedModuleSymbol, importLiteral, sourceFile, program, host, oldToNew);
157157

@@ -193,11 +193,11 @@ namespace ts {
193193
const resolved = host.resolveModuleNames
194194
? host.getResolvedModuleWithFailedLookupLocationsFromCache && host.getResolvedModuleWithFailedLookupLocationsFromCache(importLiteral.text, importingSourceFile.fileName)
195195
: program.getResolvedModuleWithFailedLookupLocationsFromCache(importLiteral.text, importingSourceFile.fileName);
196-
return getSourceFileToImportFromResolved(resolved, oldToNew, program.getSourceFiles());
196+
return getSourceFileToImportFromResolved(importLiteral, resolved, oldToNew, program.getSourceFiles());
197197
}
198198
}
199199

200-
function getSourceFileToImportFromResolved(resolved: ResolvedModuleWithFailedLookupLocations | undefined, oldToNew: PathUpdater, sourceFiles: readonly SourceFile[]): ToImport | undefined {
200+
function getSourceFileToImportFromResolved(importLiteral: StringLiteralLike, resolved: ResolvedModuleWithFailedLookupLocations | undefined, oldToNew: PathUpdater, sourceFiles: readonly SourceFile[]): ToImport | undefined {
201201
// Search through all locations looking for a moved file, and only then test already existing files.
202202
// This is because if `a.ts` is compiled to `a.js` and `a.ts` is moved, we don't want to resolve anything to `a.js`, but to `a.ts`'s new location.
203203
if (!resolved) return undefined;
@@ -210,8 +210,9 @@ namespace ts {
210210

211211
// Then failed lookups that are in the list of sources
212212
const result = forEach(resolved.failedLookupLocations, tryChangeWithIgnoringPackageJsonExisting)
213-
// Then failed lookups except package.json since we dont want to touch them (only included ts/js files)
214-
|| forEach(resolved.failedLookupLocations, tryChangeWithIgnoringPackageJson);
213+
// Then failed lookups except package.json since we dont want to touch them (only included ts/js files).
214+
// At this point, the confidence level of this fix being correct is too low to change bare specifiers or absolute paths.
215+
|| pathIsRelative(importLiteral.text) && forEach(resolved.failedLookupLocations, tryChangeWithIgnoringPackageJson);
215216
if (result) return result;
216217

217218
// If nothing changed, then result is resolved module file thats not updated
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
//// {
5+
//// "compilerOptions": {
6+
//// "allowJs": true,
7+
//// "paths": {
8+
//// "*": ["./next/src/*"],
9+
//// "@app": ["./modules/@app/*"],
10+
//// "@app/*": ["./modules/@app/*"],
11+
//// "@local": ["./modules/@local/*"],
12+
//// "@local/*": ["./modules/@local/*"]
13+
//// }
14+
//// }
15+
//// }
16+
17+
// @Filename: /modules/@app/something/index.js
18+
//// import "@local/some-other-import";
19+
20+
// @Filename: /modules/@local/index.js
21+
//// import "@local/some-other-import";
22+
23+
verify.getEditsForFileRename({
24+
oldPath: "/modules/@app/something",
25+
newPath: "/modules/@app/something-2",
26+
newFileContents: {}
27+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
6+
// @Filename: /modules/@app/something/index.js
7+
//// import "doesnt-exist";
8+
9+
// @Filename: /modules/@local/foo.js
10+
//// import "doesnt-exist";
11+
12+
verify.getEditsForFileRename({
13+
oldPath: "/modules/@app/something",
14+
newPath: "/modules/@app/something-2",
15+
newFileContents: {}
16+
});

0 commit comments

Comments
 (0)