eslint-plugin-import
eslint-plugin-import copied to clipboard
Add rule to enforce default import naming
This rule enforces a specific naming for default imports.
Fixes #1041. Closes indico/indico#3329
Coverage increased (+0.05%) to 97.959% when pulling f48590dad2b01100ead96fdeb0c4af38921f30ae on mic4ael:rename-default-import into fef718cb134016855afe61682e32c63ba3ed9281 on benmosher:master.
Updated ;)
So I guess it is not gonna be accepted unless I make it conform to the propositions listed in the linked issue?
How is this related to #1041? That issue seems to be about enforcing the same name a library uses to export its default with (e.g. export default function foo() {}
) while this one is simply about enforcing a name chosen by whoever configures eslint.
Any update on this?
Any update on this one?
Updated. Let me know what you think.
Let me know what you think now
Any update on this? ;)
Updated once again.
import * as Something from 'prop-types'
shouldn't throw any error and that is how it works. Also, I have updated my PR according to the latest comments ;)
Updated ;)
I had to fix a problem with ESLint 4 and now it works fine. Would be nice to know if there is something else to add/fix.
Hey, what is the state of this PR at the moment?
It's awaiting review from a second collaborator.
Maybe there is another collaborator that we could ask for a review?
Hm, just a casual reminder that this PR is still awaiting a review ;)
OMG, guys. You are so slow. This is a super nice rule and it takes you forever to integrate. I guess I'll just copy-paste it to my project…
@Uko the PR is waiting for a response from @mic4ael.
@mic4ael i apologize for the very long delay in getting you a review, but it's been almost as long since I've provided one :-) are you still interested in updating this PR?
@Uko the PR is waiting for a response from @mic4ael.
@mic4ael i apologize for the very long delay in getting you a review, but it's been almost as long since I've provided one :-) are you still interested in updating this PR?
hey @ljharb sorry for the delay, I will update my PR soon ;)
Thanks! I made a few tweaks and added a test case (which is unfortunately failing).
I'm not convinced on the rule name, though -
rename-default-import
is a bit weird; this seems more likeenforce-import-name
. It also kind of seems like the same rule should be preventing named imports from being renamed withas
?
Ok, I will rename it and will look into the suggestion you made.
Also, as to this:
It also kind of seems like the same rule should be preventing named imports from being renamed with as
Can you please provide an example of what exactly you mean by that?
hey @ljharb, can you please take a quick look at my responses? I would like to clarify some of the issues and (maybe) finalize this PR.
FWIW, I agree with @ljharb that the rule's name sounds like it might enforce any import name, not just the default, but I don't know that there is much value in the latter and I think the config could be made smarter if that enhancement is desired later, e.g. "prop-types": { "default": "PropTypes", "allow-renames": false | ["x", "y"] | ... }
-- so I am fine keeping enforce-import-name
but I'll defer to @ljharb.
OMG, guys. You are so slow.
Hard to argue with this 😅 since I'm generally MIA and @ljharb is a person dedicated to keeping it clean and understands the cost of adding code that you're not prepared to maintain, which I really appreciate.
Also, there is always patch-package
in a pinch!
I just looked at this rule source code and it looks like it doesn't do at all what was described in the original #1041 feature request? It was supposed to enforce the name in the import statement, but it should go to the imported file and look at the export name and enforce that.
Right now it only enforces the names which you define in the options and you have to manually list all the modules with default exports and also find out what are the names of these default exports. All this information is already available: for external packages it's in their .d.ts
file and for relative imports it's in the source code of the imported file.
I'm 100% sure it's possible to implement because VSCode can suggest autoimports based on the names of default exports in other files. Also I'm pretty sure that TSLint has a similar rule.
Ah yep, good point @phaux. also @ThiefMaster and @ljharb before you, heh. 😅 I should have re-read the source issue before chiming in at the end. (also: I didn't read enough of the following discussion to know if I'm about to just restate discussion from there 😬)
@mic4ael: is it important for your use that the rule config allow you to lint for a name that is not the original module's default export's name?
Properly solving #1041 will necessitate the original module code is available but that is typically the expected case for this plugin, and the resolver sub-plugins implement that side of it anyway (e.g. the TS resolver plugin knows where to find .d.ts
files or TS source, node resolver checks package.json
, etc.).
Also consequently, if the default export is some nameless expression, this rule would not apply.
Less config == better config, IMO. I'd be fine with allowing overrides if absolutely necessary, but not allowing them and just always reading the source for the default export's true name would be simpler.
Also consequently, if the default export is some nameless expression, this rule would not apply.
I don't think this is helpful... as a user of a library you have no influence on that. I don't know if e.g. prop-types
has a named default export... (we want it to be importet consistently as propTypes
and not PropTypes
)
Okay, so there is some desire to allow overrides then. That would support unnamed exports, false modules (e.g. CommonJS), and renaming.
Still seems like you'd want the default behavior to pick up the module's original name if it's available, though?
Bump! So, how's it going guys?