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

Add rules for import checking #425

Closed
dignifiedquire opened this issue Feb 17, 2016 · 8 comments

Comments

@dignifiedquire
Copy link

commented Feb 17, 2016

I feel this might be a great edition for everyone using es module syntax to get checks that all imports are actually resolvable: https://www.npmjs.com/package/eslint-plugin-import

what do you think?

@jprichardson

This comment has been minimized.

Copy link
Member

commented Feb 17, 2016

While I like the idea of this, Babel can be configured to dynamically resolve paths. If we did this, we would have to accommodate that as well and thus add more complexity. I'd vote to hold off on this for awhile.

@cesarandreu

This comment has been minimized.

Copy link
Contributor

commented Feb 18, 2016

The loader spec isn't finalized, so module resolution is not set in stone.

For example, how would a browser know it has to request e.g. bar.js vs bar/index.js? I think it won't even support this, so it's not clear how interop is expected to work, unless you keep a manifest?

With that said, I have no plans of giving up browserify / webpack, and will keep transpiling forever, so I don't think I'd be affected.

@dignifiedquire

This comment has been minimized.

Copy link
Author

commented Feb 18, 2016

As far as I understand it currently supports nodes resolve mechanisms, based on substacks work for browserify and webpack. That means it is capable of handling the current ways resolving is done. When the module resolution actually gets finalised and implemented I'm pretty sure it can be updated to reflect those changes.

@dignifiedquire

This comment has been minimized.

Copy link
Author

commented Feb 18, 2016

@jprichardson what dynamic resolution are you referring to exactly?

@feross

This comment has been minimized.

Copy link
Member

commented Feb 19, 2016

If we support this, then folks will be asking for hooks to customize how module resolution works for their specific setup, since not everyone uses browserify. Then there will be pressure to add hooks for those folks, or to disable these checks. I'm -1 for now, until things shake out a bit more.

@dcousens

This comment has been minimized.

Copy link
Member

commented Feb 24, 2016

This sounds like hell.

@benmosher

This comment has been minimized.

Copy link

commented Jun 13, 2016

Hello, I maintain that plugin. 😅

To support different loaders (browserify/node, Webpack, Babel plugins, etc.) the plugin itself has pluggable loaders (as @dignifiedquire noted). The vanilla Node one uses substack's node-resolve, Webpack one uses webpack/enhanced-resolve + loads your Webpack config to get aliases, etc. There are a handful of community ones as well that integrate with jspm and a few Babel import-rewriting plugins, IIRC.

So while you would not need to implement the different loading strategies, there would need to be some way to configure them, or at least choose from one that is supported by a loader. And folks using a loader that is not yet supported would need some way to indicate that, to avoid spurious warnings.

While I'm not super familiar with your project, this sounds like more configuration than you're looking for. "if you want import resolution, use ESLint directly" seems like a reasonable decision to me, considering your perspective.

That said, if you are ever interested, I'd be up for working together on some sort of zero-conf loader inference or some such. Just throwing it out there.

@feross

This comment has been minimized.

Copy link
Member

commented Jun 13, 2016

@benmosher Thanks for the explanation :)

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
6 participants
You can’t perform that action at this time.