Fixed crashes when moving namespace imports to other files in refactorings #60302
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| cache.add( | ||
| importingFile.path, | ||
| moduleSymbol, | ||
| moduleSymbol.escapedName, |
There was a problem hiding this comment.
I suspect this might be somewhat questionable solution here, I'm not fully sold on it myself either. cc @andrewbranch - you might have some opinions here (or suggestions for an improved fix? :P)
| `import * as A from "./a"; | ||
| import * as A from "./c"; |
There was a problem hiding this comment.
As we can see here, those refactorings introduce issues into target files. This is not a new problem from what I can tell. This refactoring just never tried to rename inserted symbols to avoid name clashes etc.
At first, I thought those would be test cases that should be covered by this PR so I added them. I could just remove them now but I think they still have some value here, especially if this naming conflict is ever meant to be addressed in the future.
|
I actually have a fix for this that I'm finishing the PR for right now. I'll bring your tests over since you've got some extra cases I don't have. This PR seems to be similar to #59004 with what you brought up here https://github.com/microsoft/TypeScript/pull/60302/files#r1809202658, and so module symbols should be treated differently. There's other similar crashes that aren't |
|
|
||
| function addImportFromExportedSymbol(exportedSymbol: Symbol, isValidTypeOnlyUseSite?: boolean, referenceImport?: ImportOrRequireAliasDeclaration) { | ||
| const moduleSymbol = Debug.checkDefined(exportedSymbol.parent); | ||
| const moduleSymbol = Debug.checkDefined(isExternalModuleSymbol(exportedSymbol) ? exportedSymbol : exportedSymbol.parent); |
There was a problem hiding this comment.
the crash is happening here today because the external module symbol doesn't have a .parent
|
@iisaduan cool, I missed the other PR - so I'll just close this one |
|
… On Mon, Oct 21, 2024 at 10:58 PM Mateusz Burzyński ***@***.***> wrote:
Closed #60302 <#60302>.
—
Reply to this email directly, view it on GitHub
<#60302 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AVPP67EHLLYETPMIGN2PR3LZ4U2R7AVCNFSM6AAAAABQKUQITSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJUG43DQNRWGQ3TAMQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
4 participants
cc @navya9singh @iisaduan