-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix multiline import specifier sorting #51634
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
Conversation
I was thinking about adding an internal |
Would like to get @rbuckton’s reviews for the minor changes to emit nodes and factory |
This seem to be adding trailing commas to single line imports where there was no trailing commas before. Is that intended? I think it will bring a lot upset people to file issues to see that happening, if there is no setting for that. While it's just stylistic choice, this can make a lot churn for existing code in future. However, it's possible that I'm reading test results totally wrong. |
@rubiesonthesky how are you determining that? Can you give an example? |
Fixes #51615
Whenever a codefix/refactor prints a modified node list, the emitter looks at the original nodes to determine whether to preserve newlines between the list elements. We preserve the spacing between two list elements if they were consecutive in the original list and are still consecutive in the modified list. Otherwise, we fall back to default list formatting rules for the parent node type. This logic is pretty good, but the default format for named imports/exports was single-line, so if you sort a shuffled set of named imports that were formatted one per line, you’re going to end up with a sorted list that’s mostly on a single line, but with line breaks preserved between any elements that happened to already be already sorted relative to each other. This is obviously the worst of both worlds.
The fix here is simply to say that if the original list is not single-line, the default list format should be multiline. That way, we can still preserve neighbors that are already well-sorted and grouped on a single line (as we see a couple examples of in
organizeImports1.ts
), and we can preserve any double line breaks that occur between already well-sorted imports, but any shuffles will default to getting their own line.