Do not reorder side effect imports

[blutorange]

Apologies if this has been mentioned already, I did not find any issue for, though.

Is your feature request related to a problem?

A side effect import is an import that only imports a module for its side effects, without importing any values (i.e. where ImportDeclaration.specifiers is an empty array). These are currently sorted just like any other imports. Since those imports are explicitly used for side-effects, changing the order will usually result in a different behavior.

Now side effects are something that should be avoided if possible, and library code should have side effects. However, there are some valid use cases, such as loading mocks for tests, mock-fs or loading global libraries in an index file of a wep app, etc. For example:

import "./setup-test-environment"; // setup some required globals on which some module rely unfortunately
import { something } from "./a-module-to-test";

The above would be re-ordered, which breaks the test.

Describe the solution you'd like

Side effect imports should not be reordered. eslint-plugin-import does not reorder them either, for the same reason.

Since it is hard to know which side effect imports affect which other imports, I would suggest the following solution: treat a side effect import as a boundary that splits the imports into two groups, the imports before the the side effect import and the imports after the side effect import. Then only sort the imports before and after the side effect import as you would normally. This is I think the easiest to implement and also the safest. If users want to have side effect at the top, they can just move the side effect imports to the top, and the formatter will leave them there.

I made this a feature, but I would almost consider it a bug. By default, side effect imports should not be reordered. An option could be added for sorting these imports too, but I'm not sure if that's really necessary. It also goes against prettier's philosophy of limiting the options as much as possible.

So for example:

// Side effect import 1
import "S1";

// Start group 1
import {} from "b";
import * as A from "c";
// End group 1

// Side effect import 2
import "S2";

// Start group 2
import foo from "d";
import { bar } from "e";
// End group 2

// Side effect import 3
import "S3";

In the example above, keep the side effect imports as they are. Sort the imports in group 1 and group 2 using whatever options are set. Do not move any import from group 1 to group 2 or vice-versa.

Describe alternatives you've considered

Another alternative I can think of is to simply keep the relative ordering of the side effect imports, but allow moving non side effect imports anywhere. Or always just put side effect imports at the top. Both of this possibilities have the disadvantage they the may be unsafe: putting an import from before a side effect import after it may break it because it does not expect that side effect; putting an import from after a side-effect before it may break it because it relies on the side effect. It's also unclear how imports with and without side effects should be ordered relatively to each other.

[blutorange]

Probably also solves #95, if I understand that issue correctly.

[artola]

@blutorange Same for me. We would like to use this plugin, but the ordering must be "perfect", as the users do not expect their code to break unexpectedly just because of saving a file for example.

[NiklasPor]

Same for us, I just crashed all of our unit tests because for our jest setup we need a specific import order: nrwl/nx#6361 (comment).

Also, I didn't find any workaround, except adding those files to the .prettierignore.

[artola]

@ayusharma This bug is really a showstopper, sometimes for a few files you can use @NiklasPor workaround mentioned above, but it does not scale when you run a huge codebase with many devs that should be aware, making the code really fragile.
If a test breaks, probably you will find it early, but if the change is more subtle, like CSS order, then it will go direct on your face.

[blutorange]

These issues make me wonder if anybody who's using this plugin actually has any side effect imports in files that are not in the .prettierignore. If nobody does, then #111 wouldn't even affect existing users.

[artola]

@blutorange #111 is great solution. Thanks! Now I hope to get Trivago's team support to get this bug solved.

[NiklasPor]

Actually, our workaround looks now a bit different. I just overwrote the sorting order for those specific files. Still not a great solution:

// prettierrc.js
overrides: [
    {
      files: 'test-setup.ts',
      options: {
        importOrder: ['jest.*', '<THIRD_PARTY_MODULES>'],
      },
    },
  ]

[DavidArmendariz]

Has this issue being solved?

[blutorange]

The current state of the issue, as far as I know, is that it's not a bug and also working as intended. There was the suggestion to add an option (disabled by default) that enables #111. If anybody wants to take that PR and add said option, feel free to, but personally I'm not interested in a solution that is "broken" by default. Perhaps somebody else has an idea how to solve this in a way that can be enabled by default?

[artola]

@blutorange is it time to fork?

[cvolant]

@blutorange I agree that you PR would not bother anyone, and I don't understand the maintainers point, but maybe we can proceed step by step.
The first step would be to activate the new behavior you implemented with an option and leave the default behavior as is.
Once the new behavior will have proven its worth, the next step would be to set it as the default, and an option would activate the former behavior.
If every case is handled with the new behavior, and if the option become useless, the last step would be to remove this option.

[blutorange]

Hmm, yeah, forking this project just for changing one default option is not worth the effort. If anything I was thinking about a different sorting plugin with a different philosophy (closer to prettier, opinionated, not many settings, more emphasis on correctness). But for now that's just a thought.

So yes, since this plugin already quite a few options and keeping the number of options low does not seem to be a major goal (?), one more options could be the way to go. I might add that option to the PR when I have time for it, or sometime else does it in the mean time.

Edit: If we make this an option, we can go a little step further and call it unsortableImports, which lets people define which imports should not be sorted, with the default being ["unnamed"]. This would solve issues with e.g. named CSS exports that should not be sorted.

[IanVS]

I think that #111 is an important bugfix, and that this plugin is not suitable for production use without it. So, I've published a fork which is currently the same as this package, but with #111 included (without any kind of option necessary). It can be found at https://www.npmjs.com/package/@ianvs/prettier-plugin-sort-imports for anyone else who might find it useful.

[ianldgs]

Just had some annoying CSS issues because the import of the library's CSS was moved to after our custom style overrides.
Love this library, but it doesn't make sense for us to ignore prettier completely for a file just because of the imports. Will move to the fork while this issue is not resolved.

[cvolant]

We moved too for the exact same reason.

[Morriz]

Any forward motion towards unifying these efforts? Can #111 not be merged or reworked? Or @IanVS's fork be merged into this one? It sux to have a whole fork for what is in essence a bug, as this plugin is unreliable without the fix.

@blutorange it seems you are most involved with this, so may I ask what is the blocker here?

[blutorange]

@Morriz The blocker is that the maintainers of this project consider this bug fix as "it fails the basic idea of the plugin"

[IanVS]

@Morriz FWIW, my fork now has a few features that this project does not have, and gets a fair number of npm downloads. I use it in a few of my own projects, and continue to maintain it. In case that eases your mind about using a fork.