-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[Generic Import Callback] Make callback for all import once in rules #1237
Conversation
lastNode = node.source | ||
}, | ||
|
||
CallExpression(node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here this PR changes the behavior, but according to the code, it should be a hidden bug and this change fixes it
Coverage check fails because this PR reduces sources lines, though uncovered lines don't increase... |
7245889
to
ab3a1a1
Compare
src/core/addForGenericImport.js
Outdated
CallExpression = noop, | ||
} = allCallbacks | ||
|
||
return Object.assign({}, allCallbacks, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this use object spread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but we need add the babel plugin for that. If it's ok, I can add it in this PR
https://github.com/benmosher/eslint-plugin-import/blob/798eed7e559adab2eac07bf1b3e3518d4b7a4296/.babelrc#L2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, let's do that in a separate PR, that also lints using prefer-object-spread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure :)
ab3a1a1
to
c2325fc
Compare
@ljqx i've rebased this, but some tests are failing now. could you take a look? |
This update could help with a lint task on Prettier. Is there anything we can do to help get this update finalized and ready for an upcoming release? I did take a quick look at the build failures from last year, found messages like this:
I wonder if something in the stack had become stricter over time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry it's 23 months later, but LGTM 👍🏼😅
@ljqx any chance you'd be able to rebase this? it's a bit hairy. |
c2325fc
to
298b914
Compare
Hi, Ben and Jordan, updated, and replaced the introduced helper function with existing |
298b914
to
5368cb0
Compare
This comment has been minimized.
This comment has been minimized.
The travis and coverage have passed :) |
5368cb0
to
d8fd2ba
Compare
5037470
to
93e060f
Compare
This PR fixes #1230.
Not only for
import/extensions
, this PR also adds supports of allImportDeclaration
,ExportNamedDeclaration
,ExportAllDeclaration
,CallExpression
for rules
import/extensions
,import/max-dependencies
,import/no-extraneous-dependencies
,import/no-internal-modules
,import/no-nodejs-modules
,import/no-restricted-paths
,import/no-self-import
,import/no-webpack-loader-syntax
by abstracting out the common part for these callbacks.Tests are added for
import/extensions
.