eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

Add rule to enforce default import naming

Open mic4ael opened this issue 6 years ago • 29 comments

This rule enforces a specific naming for default imports.

Fixes #1041. Closes indico/indico#3329

mic4ael avatar Jul 24 '18 06:07 mic4ael

Coverage Status

Coverage increased (+0.05%) to 97.959% when pulling f48590dad2b01100ead96fdeb0c4af38921f30ae on mic4ael:rename-default-import into fef718cb134016855afe61682e32c63ba3ed9281 on benmosher:master.

coveralls avatar Jul 24 '18 06:07 coveralls

Updated ;)

mic4ael avatar Jul 24 '18 08:07 mic4ael

So I guess it is not gonna be accepted unless I make it conform to the propositions listed in the linked issue?

mic4ael avatar Jul 30 '18 13:07 mic4ael

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.

ThiefMaster avatar Aug 03 '18 17:08 ThiefMaster

Any update on this?

mic4ael avatar Aug 30 '18 08:08 mic4ael

Any update on this one?

mic4ael avatar Sep 19 '18 07:09 mic4ael

Updated. Let me know what you think.

mic4ael avatar Sep 24 '18 06:09 mic4ael

Let me know what you think now

mic4ael avatar Sep 28 '18 10:09 mic4ael

Any update on this? ;)

mic4ael avatar Oct 04 '18 07:10 mic4ael

Updated once again.

mic4ael avatar Oct 16 '18 06:10 mic4ael

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 ;)

mic4ael avatar Oct 16 '18 09:10 mic4ael

Updated ;)

mic4ael avatar Oct 17 '18 06:10 mic4ael

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.

mic4ael avatar Oct 18 '18 10:10 mic4ael

Hey, what is the state of this PR at the moment?

mic4ael avatar Jan 07 '19 08:01 mic4ael

It's awaiting review from a second collaborator.

ljharb avatar Jan 07 '19 19:01 ljharb

Maybe there is another collaborator that we could ask for a review?

mic4ael avatar Feb 07 '19 09:02 mic4ael

Hm, just a casual reminder that this PR is still awaiting a review ;)

mic4ael avatar Apr 10 '19 12:04 mic4ael

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 avatar Apr 23 '20 12:04 Uko

@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?

ljharb avatar Jun 05 '20 22:06 ljharb

@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 ;)

mic4ael avatar Jun 06 '20 09:06 mic4ael

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 like enforce-import-name. It also kind of seems like the same rule should be preventing named imports from being renamed with as?

Ok, I will rename it and will look into the suggestion you made.

mic4ael avatar Sep 15 '20 09:09 mic4ael

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?

mic4ael avatar Sep 15 '20 13:09 mic4ael

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.

mic4ael avatar Sep 29 '20 11:09 mic4ael

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!

benmosher avatar Oct 19 '20 12:10 benmosher

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.

phaux avatar Oct 19 '20 23:10 phaux

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.

benmosher avatar Oct 27 '20 15:10 benmosher

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)

ThiefMaster avatar Oct 27 '20 16:10 ThiefMaster

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?

benmosher avatar Oct 28 '20 13:10 benmosher

Bump! So, how's it going guys?

basickarl avatar Sep 25 '23 21:09 basickarl