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

autofix "import/order" will produce new problems? #1252

Closed
abangcc opened this issue Jan 3, 2019 · 1 comment · Fixed by #1253
Closed

autofix "import/order" will produce new problems? #1252

abangcc opened this issue Jan 3, 2019 · 1 comment · Fixed by #1253

Comments

@abangcc
Copy link

abangcc commented Jan 3, 2019

env:
os: macos 10.14.2
node: 10.15.0
npm: 6.4.1
eslint: local 5.11.1

package.json

{
  "name": "testeslint",
  "version": "0.0.1",
  "scripts": {
    "eslint": "eslint --ext .js .",
  },
  "devDependencies": {
    "eslint": "^5.11.1",
    "eslint-config-airbnb-base": "^13.1.0",
    "eslint-plugin-import": "^2.14.0",
    "express": "^4.16.4"
  }
}

.eslintrc.js

module.exports = {
    "extends": "airbnb-base"
};

a.js

const env = require('./config');

Object.keys(env);

const http = require('http');

function express() {}

http.createServer(express());

b.js

const env = require('./config');

Object.keys(env);

const http = require('http');
const express = require('express');

http.createServer(express());
$ npm run eslint

> testeslint@0.0.1 eslint /Users/hbshun/testeslint
> eslint --ext .js .

/Users/hbshun/testeslint/a.js
  5:14  error  `http` import should occur before import of `./config`  import/order

/Users/hbshun/testeslint/b.js
  1:13  error  `./config` import should occur after import of `express`                       import/order
  6:17  error  'express' should be listed in the project's dependencies, not devDependencies  import/no-extraneous-dependencies

✖ 3 problems (3 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.
$ npm run eslint -- --fix

> testeslint@0.0.1 eslint /Users/hbshun/testeslint
> eslint --ext .js . "--fix"

/Users/hbshun/testeslint/a.js
  5:14  error  `http` import should occur before import of `./config`  import/order

/Users/hbshun/testeslint/b.js
  2:13  error  'env' was used before it was defined                                           no-use-before-define
  5:17  error  'express' should be listed in the project's dependencies, not devDependencies  import/no-extraneous-dependencies

✖ 3 problems (3 errors, 0 warnings)

Now, b.js:

Object.keys(env);

const http = require('http');
const express = require('express');
const env = require('./config');

http.createServer(express());

repo for test

@ljharb
Copy link
Member

ljharb commented Jan 3, 2019

This is a serious bug; if it's not fixed soon, we'll have to disable autofixing for the entire rule.

cc @tihonove / #908, @justinanastos / #1137.

tihonove added a commit to tihonove/eslint-plugin-import that referenced this issue Jan 3, 2019
tihonove added a commit to tihonove/eslint-plugin-import that referenced this issue Jan 3, 2019
Reordering import statement to line below ignores uncrossable statements
Fix import-js#1252
tihonove added a commit to tihonove/eslint-plugin-import that referenced this issue Jan 3, 2019
tihonove added a commit to tihonove/eslint-plugin-import that referenced this issue Jan 3, 2019
Reordering import statement to line below ignores uncrossable statements
Fix import-js#1252
tihonove added a commit to tihonove/eslint-plugin-import that referenced this issue Feb 17, 2019
tihonove added a commit to tihonove/eslint-plugin-import that referenced this issue Feb 17, 2019
Reordering import statement to line below ignores uncrossable statements
Fix import-js#1252
ljharb pushed a commit to tihonove/eslint-plugin-import that referenced this issue Oct 9, 2019
Fixes import-js#1252.

 - Reordering import statement to line below ignores uncrossable statements
 - Add more tests for ordering around function call
@ljharb ljharb closed this as completed in e62011f Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants