Skip to content

Commit

Permalink
[New] add no-import-module-exports rule: report import declarations…
Browse files Browse the repository at this point in the history
… with CommonJS exports

Fixes #760
  • Loading branch information
ttmarek authored and ljharb committed Apr 14, 2017
1 parent fe51583 commit 6e8f406
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 2 deletions.
6 changes: 5 additions & 1 deletion 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
- [`no-commonjs`]: Also detect require calls with expressionless template literals: ``` require(`x`) ``` ([#1958], thanks [@FloEdelmann])
- [`no-internal-modules`]: Add `forbid` option ([#1846], thanks [@guillaumewuip])
- add [`no-relative-packages`] ([#1860], [#966], thanks [@tapayne88] [@panrafal])
- add [`no-import-module-exports`] rule: report import declarations with CommonJS exports ([#804], thanks [@kentcdodds] and [@ttmarek])

### Fixed
- [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella])
Expand Down Expand Up @@ -732,6 +733,7 @@ for info on changes for earlier releases.
[`no-duplicates`]: ./docs/rules/no-duplicates.md
[`no-dynamic-require`]: ./docs/rules/no-dynamic-require.md
[`no-extraneous-dependencies`]: ./docs/rules/no-extraneous-dependencies.md
[`no-import-module-exports`]: ./docs/rules/no-import-module-exports.md
[`no-internal-modules`]: ./docs/rules/no-internal-modules.md
[`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md
[`no-named-as-default-member`]: ./docs/rules/no-named-as-default-member.md
Expand Down Expand Up @@ -908,6 +910,7 @@ for info on changes for earlier releases.
[#871]: https://github.com/benmosher/eslint-plugin-import/pull/871
[#858]: https://github.com/benmosher/eslint-plugin-import/pull/858
[#843]: https://github.com/benmosher/eslint-plugin-import/pull/843
[#804]: https://github.com/benmosher/eslint-plugin-import/pull/804
[#797]: https://github.com/benmosher/eslint-plugin-import/pull/797
[#794]: https://github.com/benmosher/eslint-plugin-import/pull/794
[#744]: https://github.com/benmosher/eslint-plugin-import/pull/744
Expand Down Expand Up @@ -1330,4 +1333,5 @@ for info on changes for earlier releases.
[@bicstone]: https://github.com/bicstone
[@guillaumewuip]: https://github.com/guillaumewuip
[@tapayne88]: https://github.com/tapayne88
[@panrafal]: https://github.com/panrafal
[@panrafal]: https://github.com/panrafal
[@ttmarek]: https://github.com/ttmarek
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Forbid default exports ([`no-default-export`])
* Forbid named exports ([`no-named-export`])
* Forbid anonymous values as default exports ([`no-anonymous-default-export`])
* Prefer named exports to be grouped together in a single export declaration ([`group-exports`])
* Enforce a leading comment with the webpackChunkName for dynamic imports ([`dynamic-import-chunkname`])
* Forbid imports with CommonJS exports ([`no-import-module-exports`])

[`first`]: ./docs/rules/first.md
[`exports-last`]: ./docs/rules/exports-last.md
Expand All @@ -109,6 +109,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
[`no-default-export`]: ./docs/rules/no-default-export.md
[`no-named-export`]: ./docs/rules/no-named-export.md
[`dynamic-import-chunkname`]: ./docs/rules/dynamic-import-chunkname.md
[`no-import-module-exports`]: ./docs/rules/no-import-module-exports.md

## `eslint-plugin-import` for enterprise

Expand Down
74 changes: 74 additions & 0 deletions docs/rules/no-import-module-exports.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# no-import-module-exports

Reports the use of import declarations with CommonJS exports in any module
except for the [main module](https://docs.npmjs.com/files/package.json#main).

If you have multiple entry points or are using `js:next` this rule includes an
`exceptions` option which you can use to exclude those files from the rule.

## Options

#### `exceptions`
- An array of globs. The rule will be omitted from any file that matches a glob
in the options array. For example, the following setting will omit the rule
in the `some-file.js` file.

```json
"import/no-import-module-exports": ["error", {
"exceptions": ["**/*/some-file.js"]
}]
```

## Rule Details

### Fail

```js
import { stuff } from 'starwars'
module.exports = thing

import * as allThings from 'starwars'
exports.bar = thing

import thing from 'other-thing'
exports.foo = bar

import thing from 'starwars'
const baz = module.exports = thing
console.log(baz)
```

### Pass
Given the following package.json:

```json
{
"main": "lib/index.js",
}
```

```js
import thing from 'other-thing'
export default thing

const thing = require('thing')
module.exports = thing

const thing = require('thing')
exports.foo = bar

import thing from 'otherthing'
console.log(thing.module.exports)

// in lib/index.js
import foo from 'path';
module.exports = foo;

// in some-file.js
// eslint import/no-import-module-exports: ["error", {"exceptions": ["**/*/some-file.js"]}]
import foo from 'path';
module.exports = foo;
```

### Further Reading
- [webpack issue #4039](https://github.com/webpack/webpack/issues/4039)
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
"is-core-module": "^1.0.2",
"minimatch": "^3.0.4",
"object.values": "^1.1.1",
"pkg-up": "^1.0.0",
"read-pkg-up": "^2.0.0",
"resolve": "^1.17.0",
"tsconfig-paths": "^3.9.0"
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const rules = {
'no-unassigned-import': require('./rules/no-unassigned-import'),
'no-useless-path-segments': require('./rules/no-useless-path-segments'),
'dynamic-import-chunkname': require('./rules/dynamic-import-chunkname'),
'no-import-module-exports': require('./rules/no-import-module-exports'),

// export
'exports-last': require('./rules/exports-last'),
Expand Down
65 changes: 65 additions & 0 deletions src/rules/no-import-module-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import minimatch from 'minimatch'
import path from 'path'
import pkgUp from 'pkg-up'

function getEntryPoint(context) {
const pkgPath = pkgUp.sync(context.getFilename())
return require.resolve(path.dirname(pkgPath))
}

module.exports = {
meta: {
docs: {
description: 'Disallow import statements with module.exports',
category: 'Best Practices',
recommended: true,
},
fixable: 'code',
schema: [
{
'type': 'object',
'properties': {
'exceptions': { 'type': 'array' },
},
'additionalProperties': false,
},
],
},
create(context) {
const importDeclarations = []
const entryPoint = getEntryPoint(context)
const options = context.options[0] || {}
let alreadyReported = false

function report(node) {
const fileName = context.getFilename()
const isEntryPoint = entryPoint === fileName
const isIdentifier = node.object.type === 'Identifier'
const hasKeywords = (/^(module|exports)$/).test(node.object.name)
const isException = options.exceptions &&
options.exceptions.some(glob => minimatch(fileName, glob))

if (isIdentifier && hasKeywords && !isEntryPoint && !isException) {
importDeclarations.forEach(importDeclaration => {
context.report({
node: importDeclaration,
message: `Cannot use import declarations in modules that export using ` +
`CommonJS (module.exports = 'foo' or exports.bar = 'hi')`,
})
})
alreadyReported = true
}
}

return {
ImportDeclaration(node) {
importDeclarations.push(node)
},
MemberExpression(node) {
if (!alreadyReported) {
report(node)
}
},
}
},
}
1 change: 1 addition & 0 deletions tests/files/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"dummy": true,
"main": "src-root/src-bar.js",
"devDependencies": {
"glob": "1.0.0",
"eslint": "2.x"
Expand Down
101 changes: 101 additions & 0 deletions tests/src/rules/no-import-module-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import path from 'path'
import { RuleTester } from 'eslint'

import { test, testFilePath } from '../utils'

const ruleTester = new RuleTester({
parserOptions: { ecmaVersion: 6, sourceType: 'module' }
})
const rule = require('rules/no-import-module-exports')

const error = {
message: `Cannot use import declarations in modules that export using CommonJS ` +
`(module.exports = 'foo' or exports.bar = 'hi')`,
type: 'ImportDeclaration',
}

ruleTester.run('no-import-module-exports', rule, {
valid: [
test({
code: `
const thing = require('thing')
module.exports = thing
`,
}),
test({
code: `
import thing from 'otherthing'
console.log(thing.module.exports)
`,
}),
test({
code: `
import thing from 'other-thing'
export default thing
`,
}),
test({
code: `
const thing = require('thing')
exports.foo = bar
`,
}),
test({
code: `
import foo from 'path';
module.exports = foo;
`,
// When the file matches the entry point defined in package.json
// See tests/files/package.json
filename: path.join(process.cwd(), 'tests/files/src-root/src-bar.js'),
}),
test({
code: `
import foo from 'path';
module.exports = foo;
`,
filename: path.join(process.cwd(), 'tests/files/some/other/entry-point.js'),
options: [{ exceptions: ['**/*/other/entry-point.js'] }],
}),
],
invalid: [
test({
code: `
import { stuff } from 'starwars'
module.exports = thing
`,
errors: [error],
}),
test({
code: `
import thing from 'starwars'
const baz = module.exports = thing
console.log(baz)
`,
errors: [error],
}),
test({
code: `
import * as allThings from 'starwars'
exports.bar = thing
`,
errors: [error],
}),
test({
code: `
import thing from 'other-thing'
exports.foo = bar
`,
errors: [error],
}),
test({
code: `
import foo from 'path';
module.exports = foo;
`,
filename: path.join(process.cwd(), 'tests/files/some/other/entry-point.js'),
options: [{ exceptions: ['**/*/other/file.js'] }],
errors: [error],
}),
],
})

0 comments on commit 6e8f406

Please sign in to comment.