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 new rule no-webpack-loader-syntax #586

Merged
merged 3 commits into from
Sep 26, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- `recommended` shared config. Roughly `errors` and `warnings` mixed together,
with some `parserOptions` in the mix. ([#402])
- `react` shared config: added `jsx: true` to `parserOptions.ecmaFeatures`.
- Added [`no-webpack-loader-syntax`] rule: forbid custom Webpack loader syntax in imports. ([#586], thanks [@fson]!)

### Breaking
- [`import/extensions` setting] defaults to `['.js']`. ([#306])
Expand Down Expand Up @@ -331,7 +332,9 @@ for info on changes for earlier releases.
[`max-dependencies`]: ./docs/rules/max-dependencies.md
[`no-internal-modules`]: ./docs/rules/no-internal-modules.md
[`no-dynamic-require`]: ./docs/rules/no-dynamic-require.md
[`no-webpack-loader-syntax`]: ./docs/rules/no-webpack-loader-syntax.md

[#586]: https://github.com/benmosher/eslint-plugin-import/pull/586
[#568]: https://github.com/benmosher/eslint-plugin-import/pull/568
[#555]: https://github.com/benmosher/eslint-plugin-import/pull/555
[#538]: https://github.com/benmosher/eslint-plugin-import/pull/538
Expand Down Expand Up @@ -488,3 +491,4 @@ for info on changes for earlier releases.
[@spalger]: https://github.com/spalger
[@preco21]: https://github.com/preco21
[@skyrpex]: https://github.com/skyrpex
[@fson]: https://github.com/fson
36 changes: 36 additions & 0 deletions docs/rules/no-webpack-loader-syntax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# no-webpack-loader-syntax

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:
Copy link
Collaborator

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 ...

```js
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).
Copy link
Collaborator

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


## Rule Details

### Fail

```js
import myModule from 'my-loader!my-module';
import theme from 'style!css!./theme.css';

var myModule = require('my-loader!./my-module');
var theme = require('style!css!./theme.css');
```

### Pass

```js
import myModule from 'my-module';
import theme from './theme.css';

var myModule = require('my-module');
var theme = require('./theme.css');
```

## When Not To Use It

If you have a project that doesn't use Webpack you can safely disable this rule.
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const rules = {
'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'),
'no-absolute-path': require('./rules/no-absolute-path'),
'no-nodejs-modules': require('./rules/no-nodejs-modules'),
'no-webpack-loader-syntax': require('./rules/no-webpack-loader-syntax'),
'order': require('./rules/order'),
'newline-after-import': require('./rules/newline-after-import'),
'prefer-default-export': require('./rules/prefer-default-export'),
Expand Down
28 changes: 28 additions & 0 deletions src/rules/no-webpack-loader-syntax.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import isStaticRequire from '../core/staticRequire'

function reportIfNonStandard(context, node, name) {
if (name.indexOf('!') !== -1) {
context.report(node, `Unexpected '!' in '${name}'. ` +
'Do not use import syntax to configure webpack loaders.'
)
}
}

module.exports = {
meta: {
docs: {},
},

create: function (context) {
return {
ImportDeclaration: function handleImports(node) {
reportIfNonStandard(context, node, node.source.value)
},
CallExpression: function handleRequires(node) {
if (isStaticRequire(node)) {
reportIfNonStandard(context, node, node.arguments[0].value)
}
},
}
},
}
74 changes: 74 additions & 0 deletions tests/src/rules/no-webpack-loader-syntax.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { test } from '../utils'

import { RuleTester } from 'eslint'

const ruleTester = new RuleTester()
, rule = require('rules/no-webpack-loader-syntax')

const message = 'Do not use import syntax to configure webpack loaders.'

ruleTester.run('no-webpack-loader-syntax', rule, {
valid: [
test({ code: 'import _ from "lodash"'}),
test({ code: 'import find from "lodash.find"'}),
test({ code: 'import foo from "./foo.css"'}),
test({ code: 'import data from "@scope/my-package/data.json"'}),
test({ code: 'var _ = require("lodash")'}),
test({ code: 'var find = require("lodash.find")'}),
test({ code: 'var foo = require("./foo")'}),
test({ code: 'var foo = require("../foo")'}),
test({ code: 'var foo = require("foo")'}),
test({ code: 'var foo = require("./")'}),
test({ code: 'var foo = require("@scope/foo")'}),
],
invalid: [
test({
code: 'import _ from "babel!lodash"',
errors: [
{ message: `Unexpected '!' in 'babel!lodash'. ${message}` },
],
}),
test({
code: 'import find from "-babel-loader!lodash.find"',
errors: [
{ message: `Unexpected '!' in '-babel-loader!lodash.find'. ${message}` },
],
}),
test({
code: 'import foo from "style!css!./foo.css"',
errors: [
{ message: `Unexpected '!' in 'style!css!./foo.css'. ${message}` },
],
}),
test({
code: 'import data from "json!@scope/my-package/data.json"',
errors: [
{ message: `Unexpected '!' in 'json!@scope/my-package/data.json'. ${message}` },
],
}),
test({
code: 'var _ = require("babel!lodash")',
errors: [
{ message: `Unexpected '!' in 'babel!lodash'. ${message}` },
],
}),
test({
code: 'var find = require("-babel-loader!lodash.find")',
errors: [
{ message: `Unexpected '!' in '-babel-loader!lodash.find'. ${message}` },
],
}),
test({
code: 'var foo = require("style!css!./foo.css")',
errors: [
{ message: `Unexpected '!' in 'style!css!./foo.css'. ${message}` },
],
}),
test({
code: 'var data = require("json!@scope/my-package/data.json")',
errors: [
{ message: `Unexpected '!' in 'json!@scope/my-package/data.json'. ${message}` },
],
}),
],
})