no-extraneous-packages should ignore invalid package names

[novemberborn]

Package names that are invalid in npm most likely use custom resolvers and won't be extraneous.

https://github.com/npm/validate-npm-package-name can be used to validate package names. I'm not sure whether legacy names should be treated as valid for the purpose of this issue.

[jfmengels]

Hi @novemberborn :)

Do you have an example of a case where this is problematic for you?

[novemberborn]

Sure. I maintain two Babel plugins which rewrite import statements: import-glob and files. An example of the latter is here:

import files from 'files:../../content/**/*.md'

I suppose this might be solved by writing a resolver for these Babel plugins (though I haven't checked whether this rule follows resolvers). However it seems to me that if we were to validate the package name (up to the first slash), then any invalid package name cannot be a valid dependency and is most likely to be use a custom resolver. In that case it won't be extraneous.

[jfmengels]

I haven't checked whether this rule follows resolvers

It does, in part.

I think a resolver here would help, though @benmosher is more knowledgeable on the subject ;)

I'm wondering whether removing anything before a : sign would help, but I don't think it does. Since tampering the file path with Webpack/Babel is way too customizable, I don't think we can do anything generic enough (though if you have ideas, would be great!). Especially in the case of your files plugin, it doesn't even link to anything resolvable, if I got it right.
A custom resolver would be the way to go I'd say, though this solution is way more cumbersome than I'd like :/

Any other ideas?

[novemberborn]

Since tampering the file path with Webpack/Babel is way too customizable, I don't think we can do anything generic enough (though if you have ideas, would be great!)

I think my generic approach would be to reject anything that looks invalid. But maybe that's too generic.

Especially in the case of your files plugin, it doesn't even link to anything resolvable, if I got it right.

Correct.

A custom resolver would be the way to go I'd say, though this solution is way more cumbersome than I'd like :/

Apologies for to yet looking into how custom resolvers behave. At their simplest they could return a module path, but perhaps they could return other information for the import rules to use. E.g. extraneous: false, extension: '.md', isModule: false, etc.

[jfmengels]

Apologies for to yet looking into how custom resolvers behave

I still haven't either, so no worries here :D

I think my generic approach would be to reject anything that looks invalid. But maybe that's too generic.

Well, in the case of this rule (haven't checked for the extensions rule), we actually try to determine the import type (see the src/core/importType file). But in your case, it is considered as an external package, not as the fallback unknown type, which it probably should, and that would fix your problem, at least for this rule. We could use unknown or a new type for anything that contains :.

If you use the order rule, then the type would be different, and if it's considered like a unknown type, then it will force you to put it at the end of your import statements list. We should probably document the unknown type (I didn't at the time as I thought I had covered every case ^^')

I'm 👍 with considered anything containing : as either unknown or a new type (custom?). What do you guys think? It will have an impact on other rules, so we'll have to look at it a bit more maybe.

[novemberborn]

I'm with considered anything containing : as either unknown or a new type (custom?). What do you guys think?

I think we should try and see where a path starts, and run everything before that through the validate-npm-package-name, rather than just solving for :.

[benmosher]

I think maybe a general ignore pattern might be good.

That, or a Babel resolver that knows how to speak to plugins. I'm thinking that might be a good future-proofing strategy to avoid continually re-implementing these and other Babel plugins as custom resolver behaviors.

[benmosher]

(Assuming that is technically feasible. It's been a while since I've looked at the Babel plugin API)

[novemberborn]

That, or a Babel resolver that knows how to speak to plugins. I'm thinking that might be a good future-proofing strategy to avoid continually re-implementing these and other Babel plugins as custom resolver behaviors.

You'd have to transpile the imports of each file, assuming you can determine the Babel config and the correct environment. Sounds tricky.

[benmosher]

I think adding a pre-resolver ignore pattern check would solve this.

You could add ^files: as a pattern and it could solve this and #341.

I like the simplicity of requiring the user to explicitly configure patterns for import overloading like this.

[justin808]

I just updated and hit this issue.

We use resolve.alias in webpack:

https://github.com/shakacode/react-webpack-rails-tutorial/blob/master/client/webpack.client.base.config.js#L49

/Users/justin/shakacode/react-on-rails/react-webpack-rails-tutorial/client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx
   12:1   error  'libs' should be listed in the project's dependencies. Run 'npm i -S libs' to add it  import/no-extraneous-dependencies

Any work around?

[benmosher]

@justin808 only //eslint-disable-line import/no-extraneous-dependencies ATM, sorry 😅

[novemberborn]

I'm finally coming back to this and it appears that the import/ignore setting is not used? The patterns I'm using in my project trigger import/no-extraneous-dependencies and import/extensions.

Adding a resolver doesn't help since it doesn't influence how the source is treated. Extending that and using resolvers in all rules may be the best solution. As I wrote back in May #340 (comment):

At their simplest they could return a module path, but perhaps they could return other information for the import rules to use. E.g. extraneous: false, extension: '.md', isModule: false, etc.

[robwise]

#340 (comment)

I just updated and hit this issue.
Any work around?

@justin808 Yes: https://www.npmjs.com/package/eslint-import-resolver-webpack

[ljharb]

I'm confused - npm's validity for package names is irrelevant. Anything that's a valid directory name in node_modules is a resolvable import, according to node's require algorithm.

Just because I can't have npm-installed a package name doesn't mean I didn't mkdir it.

[benmosher]

@novemberborn import/ignore skips parsing, so you're right, it doesn't impact those rules.

I have also arrived at what you've suggested: resolvers need to return additional metadata to classify import type. #479 is tracking this. I am hoping I will be able to spend some time on it next week.

[ljharb]

@novemberborn ^

[ljharb]

The proper way to handle this, I think, is the "ignore" setting and/or a custom resolver. Happy to reopen if that's not the case.