-
-
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
Add new rule no-webpack-loader-syntax
#586
Conversation
Webpack allows specifying loaders and their configuration inline in imports using a non-standard import syntax. This rule allows disabling this non-standard syntax in imports.
8604129
to
755eb86
Compare
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.
I usually prefer discussing a proposal first, to avoid turning down the down you've done all the work. In this case, I agree with the rule and the implementation looks good too.
I just have some suggestions concerning the formulation, and you're missing a line in the README.md.
Thanks a lot and good work! (but next time, opening an issue would probably be better ;) )
var moduleWithOneLoader = require("my-loader!./my-awesome-module"); | ||
``` | ||
|
||
This syntax is non-standard, so it couples the code using to Webpack. The recommended way to specify Webpack loader configuration is in a [Webpack configuration file](http://webpack.github.io/docs/loaders.html#loaders-by-config). |
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.
it couples the code to Webpack
|
||
Forbid Webpack loader syntax in imports. | ||
|
||
[Webpack](http://webpack.github.io) allows specifying [loaders](http://webpack.github.io/docs/loaders.html) and their configuration inline in imports using a special syntax like this: |
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.
Suggestion:
Webpack allows specifying the loaders to use in the imports source string using ...
Thanks for the review, I made the changes you suggested. I also understand why it's a good idea to open an issue for new features like this first, and I'll do it the next time I want something added :) |
LGTM, thanks :) |
LGTM too, thanks @fson for the submission (I think I'll use this, too!) and as always, thanks @jfmengels for reviewing! |
Webpack allows specifying loaders and their configuration inline in imports using a non-standard syntax in the path. This rule allows disabling imports like this.
It can be useful for people using Webpack who don't want their code to rely on this Webpack-specific feature and we can possibly use it in Create React App to fix facebook/create-react-app#733.