Skip to content

Conversation

@andrewbranch
Copy link
Member

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.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Nov 23, 2022
@a-tarasyuk
Copy link
Contributor

I was thinking about adding an internal multiLine property to NamedImports, similar to ObjectLiteralExpression, however, you came up with a better solution 👍🏻

@andrewbranch
Copy link
Member Author

Would like to get @rbuckton’s reviews for the minor changes to emit nodes and factory

@rubiesonthesky
Copy link

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.

@andrewbranch
Copy link
Member Author

@rubiesonthesky how are you determining that? Can you give an example?

@andrewbranch andrewbranch merged commit e6d7b52 into microsoft:main Nov 28, 2022
@andrewbranch andrewbranch deleted the bug/51615 branch November 28, 2022 23:35
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Organize imports doesn't keep imports in a vertical column

6 participants