Honor prettier-ignore and support prettier-ignore-start / prettier-ignore-end comments #2
Conversation
Part of trivago/prettier-plugin-sort-imports#112 Note: Due to a limitation of how the AST is generated from the import nodes, an empty lines is inserted before the range end comment: // prettier-ignore-start import "e"; import c from "c"; import { d } from "d"; // prettier-ignore-end import { f } from "f"; import { g } from "g";
blutorange
changed the title
There was a problem hiding this comment.
This is great, thanks so much. I have slight reservations with using prettier-ignore-start if that's not actually going to have an impact on prettier other than this plugin, since as you pointed out it's not implemented in prettier core. What would you think about some slightly different name, to avoid confusion, like prettier-sort-ignore-start?
And on that note, would you add a note to the docs, to explain that these comments can be used (and maybe the suggestion to use // format comments)?
Otherwise, this seems great and I think it'll be useful for folks.
| expect(importNodes.length).toBe(2); | ||
| expect(getChunkTypeOfNode(importNodes[0])).toBe(chunkTypeUnsortable); | ||
| expect(getChunkTypeOfNode(importNodes[1])).toBe(chunkTypeOther); | ||
| }); |
There was a problem hiding this comment.
Nice-to-have:
Do you mind adding some tests for type imports, including the new typescript 4.5 syntax: import {type Foo} from './foo';? From reading the code they should work fine, but might be nice to have explicit tests.
There was a problem hiding this comment.
I was half-way done with implementing ranged comments when I realized prettier doesn't actually support that for JavaScript. We can change the name, or I'd also not be opposed to removing support for ranged comments altogether. The more we follow the standard the better. Normal single comments can do the job as well, and there's probably something else wrong when people need to ignore half of the imports.
As for type imports, yeah, shouldn't affect it, but I can add some tests.
There was a problem hiding this comment.
I'll leave the decision up to you. Personally I think that you're right, ranged comments might not be needed. It's less convenient to need a comment above every input, but this should be a rare escape-hatch anyway. If we don't support ranges, it avoids some extra complexity, is easier to explain, and probably improves performance a tiny bit.
There was a problem hiding this comment.
Ok, then I'll remove it. It makes the code more complex too and harder to understand.
For later, I was thinking about adding an option unsortableImports that let's users specify a pattern and if an import matches it, it is considered unsortable as well. This might be required for some non-standard usages of ES module imports that were mentioned in the issues over at trivago/prettier-plugin-sort-imports. If we do that, that's probably more helpful in the long term and doesn't require manual comments.
There was a problem hiding this comment.
Sounds good, let's see how far we can get with simple ignore statements, and if there's a demand we can add more features.
This reverts commit 867b918.
…comments" This reverts commit 838a48c.
* I incremented the version of the prettier dev-dependency so that
we can test imports with type modifiers on a named symbol, e.g.
import { type foo, bar } from "./a". Since this is a dev dependency,
this should not affect users.
* Added a note to the readme regarding ignore comments.
* Added tests for ignore comments on type imports.
As I mentioned, by reusing the logic for side-effect imports, we can now implement ignore comments for trivago/prettier-plugin-sort-imports#112. Imports marked with
// prettier-ignoreare treated like side-effect imports in the sense that they represent unsortable imports. Only imports before and after those are sorted.Technically,
prettier-ignore-startandprettier-ignore-endare not supported for JavaScript by prettier, but it appears the demand is there prettier/prettier#5287.