-
-
Notifications
You must be signed in to change notification settings - Fork 44
Description
prettier-plugin-organize-imports doesn't work when using prettier-plugin-merge. The latter allows multiple plugins to format the same files. For example, plain Prettier doesn't work if you want to run both prettier-plugin-tailwindcss and prettier-plugin-classnames. With prettier-plugin-merge this is possible.
The same could be done with prettier-plugin-organize-imports and those two (or others). Except it doesn't work. prettier-plugin-organize-imports ignores the input when prettier-plugin-merge is also loaded.
I did a bit of digging and found that this comes from the range check. I read through #110, #112 and the discussion on WebStorm's YouTrack. I've repeated the tests there with and without the merge plugin. The merge plugin always includes the full code as originalText.
| Merge plugin | Range end in CLI | Invocation | rangeEnd | originalText | code |
|---|---|---|---|---|---|
| No | No | 1/1 | code.length |
undefined | Full code |
| No | Yes (e.g. 14) | 1/2 | e.g. 14 | undefined | Full code |
| 2/2 | Infinity |
Full code | Snippet | ||
| Yes | No | 1/1 | code.length |
Full code | Full code |
| Yes | Yes (e.g. 14) | 1/1 | code.length |
Full code | Full code |
When using a range end without the merge plugin, the organize-imports plugin is invoked twice. It seems Prettier does this to adjust the range to a range-after-preprocessing. The merge plugin calls only once, but doesn't update the code. (It doesn't support range formatting: ony3000/prettier-plugin-merge#21). Since there's nothing meaningful organize-imports can do with range formatting (as noted in #110), this is fine.
From the table, I conclude that the isRange Boolean in
prettier-plugin-organize-imports/index.js
Lines 25 to 28 in bc4d8fa
| const isRange = | |
| Boolean(options.originalText) || | |
| options.rangeStart !== 0 || | |
| (options.rangeEnd !== Infinity && options.rangeEnd !== code.length); |
rangeEnd to code.length. Though it stands to reason that rangeStart should also be used. Comparing both seems to make sense. If the range is the entire code, it's not really a range formatting, but rather an entire file formatting.
Crucially, it's the check for the presence of originalText that makes merge and organize-imports incompatible. From the table, I think we can do without, by changing the lines above to:
const isRange = options.rangeStart !== 0 || options.rangeEnd !== code.length;The rangeEnd comparison with Infinity can be removed as well, because code.length will always be a finite positive integer (or 0).
I tested this with the CLI and IntelliJ IDEA, and it works. I've created a PR that includes test cases too: #156