-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
getEditsForFileRename: Test both before and after the rename #25074
Conversation
ae7e48a
to
f2bbb3a
Compare
f2bbb3a
to
a5a0676
Compare
export function getModuleSpecifier(compilerOptions: CompilerOptions, fromSourceFile: SourceFile, fromSourceFileName: string, toFileName: string, host: ModuleSpecifierResolutionHost, preferences: ModuleSpecifierPreferences = {}) { | ||
const info = getInfo(compilerOptions, fromSourceFile, fromSourceFileName, host); | ||
return getGlobalModuleSpecifier(toFileName, info, host, compilerOptions) || | ||
// Note: importingSourceFile is just for usesJsExtensionOnImports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should that instead be a boolean flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do that in #25073 which is making this configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sheetalkamat can you take a look
@@ -16,6 +16,9 @@ | |||
//// "exclude": ["old"], | |||
////} | |||
|
|||
// @Filename: /src/old/someFile.ts | |||
//// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change it seems we need to have oldFile present on the disk? What about the tests when getEditsForRename when oldFile isnt present on the disk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For getEditsForFileRename
to work, either oldFile xor newFile should be present. The tests start with oldFile present, verify the output of getEditsForFileRename
, then rename the file, and verify the output of getEditsForFileRename
again.
Fixes #25059 and #25029
Updates the fourslash tests to run once before the rename, perform the rename, then run again. The bug in #25059 only reproduced if
getEditsForFileRename
was called after the rename, which we weren't testing before.