Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some side-effect imports are not autofixable #34

Closed
izhan opened this issue Jan 15, 2020 · 7 comments
Closed

Some side-effect imports are not autofixable #34

izhan opened this issue Jan 15, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@izhan
Copy link

izhan commented Jan 15, 2020

Hi there - first off, thank you for creating this project!

We are having autofix issues when running eslint --fix on this particular set of side-effect imports:

import "codemirror/addon/fold/brace-fold"
import "codemirror/addon/edit/closebrackets"
import "codemirror/addon/fold/foldgutter"
import "codemirror/addon/fold/foldgutter.css"
import "codemirror/addon/lint/json-lint"
import "codemirror/addon/lint/lint"
import "codemirror/addon/lint/lint.css"
import "codemirror/addon/scroll/simplescrollbars"
import "codemirror/addon/scroll/simplescrollbars.css"
import "codemirror/lib/codemirror.css"
import "codemirror/mode/javascript/javascript"

Expected behavior:

  • no errors are emitted
  • side-effect imports are not reordered

Actual behavior:

  • error (Run autofix to sort...)
  • side-effect imports are reordered like below
import "codemirror/addon/lint/lint"
import "codemirror/addon/fold/brace-fold"
import "codemirror/addon/fold/foldgutter"
import "codemirror/addon/fold/foldgutter.css"
import "codemirror/addon/lint/json-lint"
import "codemirror/addon/edit/closebrackets"
import "codemirror/addon/lint/lint.css"
import "codemirror/addon/scroll/simplescrollbars"
import "codemirror/addon/scroll/simplescrollbars.css"
import "codemirror/lib/codemirror.css"
import "codemirror/mode/javascript/javascript"

If I find an easier repro, I'll include it. Happy to also help dig into this further when I have a chance. Cheers.

@lydell
Copy link
Owner

lydell commented Jan 15, 2020

Hi! I pasted those imports quickly into a project that uses eslint-plugin-simple-import-sort and their order was preserved. What am I missing? There must be something different in your environment. Could you please make a reproduction repo? Try to remove stuff until it doesn’t happen anymore etc.

@izhan
Copy link
Author

izhan commented Jan 15, 2020

Weird, let me continue to dig into further...thanks for the quick reply!

@izhan
Copy link
Author

izhan commented Jan 15, 2020

Here's a repro: https://github.com/izhan/eslint-plugin-simple-import-sort-repro. Running npm run test should produce an error for failing-import.js

Interestingly, deleting any line in failing-import.js will cause the autofixer to work again.

@lydell
Copy link
Owner

lydell commented Jan 15, 2020

Thanks! npm run test produces no errors (and no diff) for me though:

~/stuff/eslint-plugin-simple-import-sort-repro master
❯ npm test

> [email protected] test /Users/lydell/stuff/eslint-plugin-simple-import-sort-repro
> eslint --fix ./failing-import.js


~/stuff/eslint-plugin-simple-import-sort-repro master
❯

@izhan
Copy link
Author

izhan commented Jan 15, 2020

Ah crap...

$ node --version
v10.16.3

Looks like stable sorting was released in Node v12+ (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort). Updating to v12.13.1 solved this issue for me.

Should we change the minimum Node version for this repo? https://github.com/lydell/eslint-plugin-simple-import-sort#development Or perhaps just implement a stable sort for sort.js?

@lydell
Copy link
Owner

lydell commented Jan 16, 2020

Good catch! I think we should support Node 10. It might be as easy as changing 0 to itemA.index - itemB.index here:

@lydell
Copy link
Owner

lydell commented Jan 24, 2020

Released in v5.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants