trivago / prettier-plugin-sort-imports

A prettier plugin to sort imports in typescript and javascript files by the provided RegEx order.
Apache License 2.0
3.23k stars 128 forks source link

sort imports does ignore prettier-ignore statements #112

Open ivanmcgregor opened 2 years ago

ivanmcgregor commented 2 years ago

Your Environment

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

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.

blutorange commented 2 years ago

@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

ivanmcgregor commented 2 years ago

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. 👍

blutorange commented 2 years ago

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):

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.

denik1981 commented 2 years ago

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.

blutorange commented 2 years ago

@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".

denik1981 commented 2 years ago

@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.

braianj commented 2 years ago

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

Morriz commented 2 years ago

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

tjkohli commented 2 years ago

+1 this is preventing us from using as a team

braianj commented 2 years ago

Just use ESLint with sorting plugin

denik1981 commented 2 years ago

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.

Dezzymei commented 1 year ago

Is there a fix for this that I am missing?

ahmafi commented 1 year ago

This is also stopping me from using this plugin.

Iuriy-Budnikov commented 1 year ago

I have the same issue with ordering in tests

felixcatto commented 1 year ago

Any progress? :cry:

gentrithalili1 commented 1 year ago

+1

viet-nv commented 11 months ago

any update? :'(

lianghx-319 commented 11 months ago

any update?

Zehelein commented 9 months ago

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).

a613 commented 5 months ago

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

AntonioLuisME commented 4 months ago

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.

ms-dosx86 commented 2 months ago

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

Worked for me, thanks!

niko7o commented 1 month ago

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.

It has to be set at the top of the file as first line, that got it to work for me - in case that helps someone else!