moveToNewFile: handle namespace imports too#38377
moveToNewFile: handle namespace imports too#38377elibarzilay merged 1 commit intomicrosoft:masterfrom
Conversation
| case SyntaxKind.ImportEqualsDeclaration: | ||
| case SyntaxKind.ImportSpecifier: | ||
| case SyntaxKind.ImportClause: | ||
| case SyntaxKind.NamespaceImport: |
There was a problem hiding this comment.
I thought about adding a comment given that NamespaceImport is in this list but NamedImports, but I'm half-guessing that it would be semi-obvious for most people, and I don't know if there's any reference for the list of syntax kinds.
There was a problem hiding this comment.
I actually don't know why this is - is it because it's somehow handled by ImportClause?
There was a problem hiding this comment.
@DanielRosenwasser, my guess is that one of NamedImports or ImportClause is for the whole import and the other is for each name in the import, or something similar (since the first is plural). But I don't want to add a comment with a guess... I did see that adding NamedImports doesn't break any tests, but went with the smaller change.
There was a problem hiding this comment.
That decision makes sense, thanks!
|
@DanielRosenwasser appetite for taking this to 3.9 final instead of waiting for a servicing release, given the diff? Will queue up a port PR either way |
|
@typescript-bot cherry-pick this to release-3.9 |
|
Heya @RyanCavanaugh, I've started to run the task to cherry-pick this into |
|
Hey @RyanCavanaugh, I've opened #38386 for you. |
Component commits: 4baff1b moveToNewFile: handle namespace imports too Fixes microsoft#35591
|
@typescript-bot pack this |
|
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 4baff1b. You can monitor the build here. |
|
Looks reasonable for 3.9.2. I'll merge it into |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build. |
Fixes #35591