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

Avoid flow imports in no-cycle rule #1231

Closed
wants to merge 1 commit into from

Conversation

claramunt
Copy link

@claramunt claramunt commented Nov 14, 2018

Avoid flow imports in no-cycle rule
Currently no-cycle rule is not working with babel parser, errors are being skipped due to

if (sourceNode._babelType === 'Literal') {	
  return // no Flow import resolution, workaround for ESLint < 5.x	
}

fixes #1166

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 97.354% when pulling f67bee4 on claramunt:master into db471a8 on benmosher:master.

@coveralls
Copy link

coveralls commented Nov 14, 2018

Coverage Status

Coverage increased (+0.001%) to 97.804% when pulling 52dbd0b on claramunt:master into 15e5c61 on benmosher:master.

@claramunt claramunt closed this Nov 14, 2018
@claramunt claramunt reopened this Nov 14, 2018
@claramunt claramunt closed this Nov 14, 2018
@claramunt claramunt reopened this Nov 14, 2018
@ljharb
Copy link
Member

ljharb commented Nov 14, 2018

Please don’t close and reopen to kick CI; maintainers can rerun failed builds if needed.

@vikr01
Copy link
Contributor

vikr01 commented Nov 16, 2018

Already fixed in #1218.

@ljharb
Copy link
Member

ljharb commented Nov 16, 2018

@vikr01 your PR has fewer code changes, and has appveyor tests failing; this one has decreased coverage, but tests are passing. Can you update yours to include all the test cases here, and see about getting everything green?

@vikr01
Copy link
Contributor

vikr01 commented Nov 16, 2018

The only new things this PR seems to add are ExportMap.js#L396 and no-cycle.js#L53/54, which are only used by ExportAllDeclaration and ImportDeclaration -- and are not covered by the tests.

Everything else added, including the tests, were copied from #1218.

@ljharb
Copy link
Member

ljharb commented Jun 6, 2019

@claramunt since #1218 is merged, I've rebased this - the only changes left have no test coverage. Could you add some?

@claramunt
Copy link
Author

Sorry for the delay, didn't see the thread. I'm closing this PR since it seems fixed in other merged PR

@claramunt claramunt closed this Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

no-cycle rule not working with babel-eslint parser
4 participants