diff --git a/CHANGELOG.md b/CHANGELOG.md index 486c0248ab..ac420f24c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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]) @@ -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 @@ -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 @@ -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 \ No newline at end of file +[@panrafal]: https://github.com/panrafal +[@ttmarek]: https://github.com/ttmarek \ No newline at end of file diff --git a/README.md b/README.md index e08e72ffa2..7945546a6c 100644 --- a/README.md +++ b/README.md @@ -67,11 +67,13 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a * Report CommonJS `require` calls and `module.exports` or `exports.*`. ([`no-commonjs`]) * Report AMD `require` and `define` calls. ([`no-amd`]) * No Node.js builtin modules. ([`no-nodejs-modules`]) +* Forbid imports with CommonJS exports ([`no-import-module-exports`]) [`unambiguous`]: ./docs/rules/unambiguous.md [`no-commonjs`]: ./docs/rules/no-commonjs.md [`no-amd`]: ./docs/rules/no-amd.md [`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md +[`no-import-module-exports`]: ./docs/rules/no-import-module-exports.md ### Style guide diff --git a/docs/rules/no-import-module-exports.md b/docs/rules/no-import-module-exports.md new file mode 100644 index 0000000000..8131fd5f78 --- /dev/null +++ b/docs/rules/no-import-module-exports.md @@ -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) diff --git a/package.json b/package.json index 654e7fe1b0..70afd11721 100644 --- a/package.json +++ b/package.json @@ -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" diff --git a/src/index.js b/src/index.js index 17002337e4..7fa3710d64 100644 --- a/src/index.js +++ b/src/index.js @@ -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'), diff --git a/src/rules/no-import-module-exports.js b/src/rules/no-import-module-exports.js new file mode 100644 index 0000000000..29d2c63c35 --- /dev/null +++ b/src/rules/no-import-module-exports.js @@ -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) + } + }, + } + }, +} diff --git a/tests/files/package.json b/tests/files/package.json index 0ca8e77737..67cea40563 100644 --- a/tests/files/package.json +++ b/tests/files/package.json @@ -1,5 +1,6 @@ { "dummy": true, + "main": "src-root/src-bar.js", "devDependencies": { "glob": "1.0.0", "eslint": "2.x" diff --git a/tests/src/rules/no-import-module-exports.js b/tests/src/rules/no-import-module-exports.js new file mode 100644 index 0000000000..dc103b7d30 --- /dev/null +++ b/tests/src/rules/no-import-module-exports.js @@ -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], + }), + ], +})