prettier-plugin-sort-imports icon indicating copy to clipboard operation
prettier-plugin-sort-imports copied to clipboard

sort imports does ignore prettier-ignore statements

Open ivanmcgregor opened this issue 3 years ago • 23 comments

Your Environment

  • Prettier version: 2.4.1 / 2.2.1 (both in yarn lock)
  • node version: v17.0.1
  • package manager: yarn@2
  • IDE: Webstorm
  • **prettier-plugin-sort-import: 3.1.1 Describe the bug

To Reproduce

Add the prettier config and add a new js or ts file. There add one import to the top that does not belong to the top usually and try to make it stay at the top via // prettier-ignore (and variants).

Expected behavior When ordering imports I would expect it to be possible to ignore imports that need to be at a certain position (e.g. why-did-you-render wanting to be imported at the top). The prettier-ignore should not be ignored.

Screenshots, code sample, etc

// prettier-ignore-start
import "../../scripts/wdyr"; // must be first import!
// prettier-ignore-end

import { ApolloProvider } from "@apollo/client";
import React from "react";

turns into

// prettier-ignore-start
// must be first import!
// prettier-ignore-end
import { ApolloProvider } from "@apollo/client";
import React from "react";

import "../../scripts/wdyr";

and

// prettier-ignore
import "../../scripts/wdyr"; // must be first import!

import { ApolloProvider } from "@apollo/client";
import React from "react";

turns into

// prettier-ignore
// must be first import!
import { ApolloProvider } from "@apollo/client";
import React from "react";

import "../../scripts/wdyr";

As can be seen, the first import does not stay the first import.

Configuration File (cat .prettierrc, prettier.config.js, .prettier.js)

module.exports = {
  importOrder: ["^@/(.*)$", "^[./]"],
  importOrderSeparation: true,
};

Error log No log.

Contribute to @trivago/prettier-plugin-sort-imports

  • [x] I'm willing to fix this bug 🥇 (if provided a bit of guidance where to look)

PS: I think I will add an alias @scripts and change the sorting for it to appear at the top. Then maybe importing @scripts/wdyr will stay.

ivanmcgregor avatar Nov 17 '21 09:11 ivanmcgregor

@ivanmcgregor The first import seems to be a side effect import, for which I made an isue here #110 and a PR here #111

I had the same issue with test files, these imports really should not be reordered, as their only purpose is side effects. I'd also advocate making not sorting side effect imports the default, without requiring manual comments.

This should solve the case you mentioned, but I guess we could discuss here whether it's still necessary to support turning off the sorting for arbitrary imports via comments.

While making the changes for #111 I also had the idea that we could use the same mechanism for this feature -- treat import declarations with an ignore comment the same as a side effect import and do not reorder them.

That said, if this does get implemented, one question that comes to my mind is: how should imports before and after the ignore section be treated?

import z2 from "z2";
import z1 from "z1";
// prettier-ignore-start
import y from "y";
// prettier-ignore-end
import x2 from "x2";
import x1 from "x1";

Obviously x1 should come before x2 and z1 before z2, but should the x's be moved before the z's (and if so, should they now be before or after the ignore section)?

If we used the same mechanism as in my PR, the z's would stay above the ignore section and the x's below it, which imo would make sense and be easy to understand

blutorange avatar Nov 17 '21 12:11 blutorange

Seems that I missed that when scanning for similar tickets to my bug, thanks for mentioning it. Although I would agree that your described behavior matches my expected behavior and would alleviate my issue here, I feel that a plugin should not break with default behavior of the parent. On the other hand I do not know how well prettier passes along things like those ignore statements so that they do not have to be reimplemented.

You did raise a valid point and after checking out your issue and PR I guess you would side with you on treating the ignore as a separate section. 👍

ivanmcgregor avatar Nov 17 '21 15:11 ivanmcgregor

Most cases I hope should be solved by not reordering side effect imports :) But yes, I agree, prettier plugins should follow prettier conventions, if possible.

Speaking of conventions, though, when I read the docs, there's two options for turning of formatting (I've only recently started using prettier, so I might be wrong about this):

  • use a //prettier-ignore comment to turn of formatting for a syntax node. This doesn't really apply here because reordering imports is not formatting a single node, it is a global modification over multiple nodes.
  • use a ranged ignore comment <!-- prettier-ignore-start --> and <!-- prettier-ignore-end -->. But in the docs they make this restriction: This type of ignore is only allowed to be used in top-level and aimed to disable formatting for auto-generated content, e.g. all-contributors, markdown-toc, etc. Which I'm not quite sure what to make of, it reads a bit like a header at the top of a file that prettier just cuts of before applying the formatter. There's also this issue about ranged ignores for JS that's still open. On the online playground, ranged comments also do not seem to be working for JavaScript

Also, looking at the plugin interface, prettier does not seems to pass any info about ignore ranges to plugins. So this would have to be implemented from scratch. Which wouldn't be terribly hard, since comments are already processed by this plugin to keep them at the correct import declaration. But it's something to consider when deciding whether it should be implemented.

But yeah, one major difference between reordering imports and "normal" formatting is that reordering imports is a global modifications over multiple nodes, so treating it as a section might be the best solution if this gets implemented.

blutorange avatar Nov 17 '21 17:11 blutorange

Not solved yet?

I'm running prettier-sort-imports in conjunction with playwright which uses a global test file aimed to determine the order of tests The order is defined by the order in how the side-effects imports for each test are arranged in this file. I'm not able to ignore the plugin nor by inline comments neither on the prettierignore file.

denik1981 avatar Mar 16 '22 07:03 denik1981

@denik1981 If it's only the side-effect imports for which you want to preserve the order, you could try this fork https://www.npmjs.com/package/@ianvs/prettier-plugin-sort-imports which includes the PR I made that was not accepted. (Seeing that many people seem to be affected, any chance that PR can get merged? Possibly by incrementing the major version?)

Also, based on how that PR handles side effects, it would be possible to implement ignore comments by marking imports with such a comment as a "side-effect import".

blutorange avatar Mar 16 '22 08:03 blutorange

@blutorange Yes, I have the thread on the PR. Shame this is not considered a bug because it is indeed a critical one. Thanks for your contribution.

denik1981 avatar Mar 16 '22 13:03 denik1981

@ayusharma Do you have an estimate of when it's going to fix something as important as this?

braianj avatar May 30 '22 20:05 braianj

yeah, too bad we can't use this plugin without this fix ;(

Morriz avatar Jun 02 '22 09:06 Morriz

+1 this is preventing us from using as a team

tjkohli avatar Jun 14 '22 14:06 tjkohli

Just use ESLint with sorting plugin

braianj avatar Jun 14 '22 19:06 braianj

Just use ESLint with sorting plugin

I finally ended up using that, and I encourage people to use it instead. This project seems to be unmaintained.

denik1981 avatar Jun 15 '22 20:06 denik1981

Is there a fix for this that I am missing?

Dezzymei avatar Dec 14 '22 10:12 Dezzymei

This is also stopping me from using this plugin.

ahmafi avatar Mar 23 '23 17:03 ahmafi

I have the same issue with ordering in tests

Iuriy-Budnikov avatar May 03 '23 10:05 Iuriy-Budnikov

Any progress? :cry:

felixcatto avatar Jun 03 '23 10:06 felixcatto

+1

gentrithalili1 avatar Jun 08 '23 09:06 gentrithalili1

any update? :'(

viet-nv avatar Sep 12 '23 10:09 viet-nv

any update?

lianghx-319 avatar Oct 10 '23 04:10 lianghx-319

I found a "decently nice" workaround by adding // eslint-disable-next-line prettier/prettier to the next line:

For example, defining the dotenv paths to search for ENV variables must run before the Config is initialized:

import dotenv from 'dotenv';
dotenv.config({ path: path.join(__dirname, '.env') });
// eslint-disable-next-line prettier/prettier
dotenv.config({ path: path.join(__dirname, '../.env'), override: true });
// eslint-disable-next-line prettier/prettier
import { Config } from './config';

Not the nicest solution but one that works (for me).

Zehelein avatar Nov 16 '23 11:11 Zehelein

The best solution I found was to disable ordering in the entire file. This works for me since I only have 1 file that needs the exception.

https://github.com/trivago/prettier-plugin-sort-imports/blob/main/docs/TROUBLESHOOTING.md#q-how-can-i-prevent-the-plugin-from-reordering-specific-import-statements

adding the following comment at the top of your file: // sort-imports-ignore

a613 avatar Mar 15 '24 22:03 a613

The best solution I found was to disable ordering in the entire file. This works for me since I only have 1 file that needs the exception.

https://github.com/trivago/prettier-plugin-sort-imports/blob/main/docs/TROUBLESHOOTING.md#q-how-can-i-prevent-the-plugin-from-reordering-specific-import-statements

adding the following comment at the top of your file: // sort-imports-ignore

This doesn't work for me.

AntonioLuisME avatar Apr 15 '24 13:04 AntonioLuisME