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

no-cycle rule not working with babel-eslint parser #1166

Closed
jer-sen opened this issue Aug 28, 2018 · 5 comments · Fixed by #1218
Closed

no-cycle rule not working with babel-eslint parser #1166

jer-sen opened this issue Aug 28, 2018 · 5 comments · Fixed by #1218

Comments

@jer-sen
Copy link

jer-sen commented Aug 28, 2018

Here is a simple repro:

package.json

{
  "dependencies": {
    "babel-eslint": "^9.0.0",
    "eslint": "^5.4.0",
    "eslint-plugin-import": "^2.14.0"
  }
}

.eslintrc

{
	parser: "babel-eslint",
	parserOptions: {
		sourceType: 'module',
	},
	plugins: ['import'],
	rules: { 'import/no-cycle': 'error' },
}

a.js

import B from './b';
export default "A" + B;

b.js

import A from './a';
export default A + "B";

No error is found by eslint but if default parser is used (by removing line parser: "babel-eslint",) then no-cycle errors are found in each file.

Is it the expected behaviour ? An issue with eslint-plugin-import ? An issue with babel-eslint ?

@edmorley
Copy link

edmorley commented Sep 25, 2018

I encountered this too.

Inspecting sourceNode in the babel-parser case, _babelType is 'Literal', which means this rule is being skipped by:
https://github.com/benmosher/eslint-plugin-import/blob/e8954dbaacd9590a8c46e3fc8ba31056576302cd/src/rules/no-cycle.js#L37-L39

If I comment that return out, then the no-cycle errors are reported as expected.

It was added in #1126 to fix #1098, though perhaps is not needed for ESLint 5 any more?
@gajus I don't suppose you can remember why it was needed? :-)

@gajus
Copy link
Contributor

gajus commented Sep 25, 2018

If I comment that return out, the the no-cycle errors are reported as expected.

It was added for support of ESLint <5.

@edmorley
Copy link

Ok, but the rule is now being skipped for ESLint 5 too. Should we just version-check, or can you remember more specifically what was missing, so that we can feature detect that instead?

@gajus
Copy link
Contributor

gajus commented Sep 25, 2018

Do not remember more specifically.

@bora89
Copy link

bora89 commented Nov 28, 2018

Hey guys, any progress on this? We experience the same issue, with babel parser no-cycle does not work

ljharb pushed a commit to vikr01/eslint-plugin-import that referenced this issue Apr 12, 2019
 - fix import() to work with no-cycle
 - add tests using multiple imports in no-cycle

Fixes import-js#1035. Fixes import-js#1166.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants