Skip to content

Commit

Permalink
Merge pull request #721 from robertrossmann/prefer-single-export
Browse files Browse the repository at this point in the history
New: `group-exports` rule
  • Loading branch information
ljharb committed Oct 31, 2017
2 parents a5844d5 + 365dfbb commit eca7b4d
Show file tree
Hide file tree
Showing 6 changed files with 413 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## [Unreleased]

- Add [`group-exports`] rule: style-guide rule to report use of multiple named exports ([#721], thanks [@robertrossmann])

## [2.8.0] - 2017-10-18
### Added
Expand Down Expand Up @@ -431,6 +432,7 @@ for info on changes for earlier releases.
[`unambiguous`]: ./docs/rules/unambiguous.md
[`no-anonymous-default-export`]: ./docs/rules/no-anonymous-default-export.md
[`exports-last`]: ./docs/rules/exports-last.md
[`group-exports`]: ./docs/rules/group-exports.md

[`memo-parser`]: ./memo-parser/README.md

Expand All @@ -442,6 +444,7 @@ for info on changes for earlier releases.
[#744]: https://github.com/benmosher/eslint-plugin-import/pull/744
[#742]: https://github.com/benmosher/eslint-plugin-import/pull/742
[#737]: https://github.com/benmosher/eslint-plugin-import/pull/737
[#721]: https://github.com/benmosher/eslint-plugin-import/pull/721
[#712]: https://github.com/benmosher/eslint-plugin-import/pull/712
[#696]: https://github.com/benmosher/eslint-plugin-import/pull/696
[#685]: https://github.com/benmosher/eslint-plugin-import/pull/685
Expand Down Expand Up @@ -661,3 +664,4 @@ for info on changes for earlier releases.
[@mplewis]: https://github.com/mplewis
[@rosswarren]: https://github.com/rosswarren
[@alexgorbatchev]: https://github.com/alexgorbatchev
[@robertrossmann]: https://github.com/robertrossmann
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Forbid unassigned imports ([`no-unassigned-import`])
* Forbid named default exports ([`no-named-default`])
* Forbid anonymous values as default exports ([`no-anonymous-default-export`])
* Prefer named exports to be grouped together in a single export declaration ([`group-exports`])

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

## Installation

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

Reports when named exports are not grouped together in a single `export` declaration or when multiple assignments to CommonJS `module.exports` or `exports` object are present in a single file.

**Rationale:** An `export` declaration or `module.exports` assignment can appear anywhere in the code. By requiring a single export declaration all your exports will remain at one place, making it easier to see what exports a module provides.

## Rule Details

This rule warns whenever a single file contains multiple named export declarations or multiple assignments to `module.exports` (or `exports`).

### Valid

```js
// A single named export declaration -> ok
export const valid = true
```

```js
const first = true
const second = true

// A single named export declaration -> ok
export {
first,
second,
}
```

```js
// A single exports assignment -> ok
module.exports = {
first: true,
second: true
}
```

```js
const first = true
const second = true

// A single exports assignment -> ok
module.exports = {
first,
second,
}
```

```js
function test() {}
test.property = true
test.another = true

// A single exports assignment -> ok
module.exports = test
```


### Invalid

```js
// Multiple named export statements -> not ok!
export const first = true
export const second = true
```

```js
// Multiple exports assignments -> not ok!
exports.first = true
exports.second = true
```

```js
// Multiple exports assignments -> not ok!
module.exports = {}
module.exports.first = true
```

```js
// Multiple exports assignments -> not ok!
module.exports = () => {}
module.exports.first = true
module.exports.second = true
```

## When Not To Use It

If you do not mind having your exports spread across the file, you can safely turn this rule off.
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const rules = {
'extensions': require('./rules/extensions'),
'no-restricted-paths': require('./rules/no-restricted-paths'),
'no-internal-modules': require('./rules/no-internal-modules'),
'group-exports': require('./rules/group-exports'),

'no-named-default': require('./rules/no-named-default'),
'no-named-as-default': require('./rules/no-named-as-default'),
Expand Down
98 changes: 98 additions & 0 deletions src/rules/group-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
const meta = {}
/* eslint-disable max-len */
const errors = {
ExportNamedDeclaration: 'Multiple named export declarations; consolidate all named exports into a single export declaration',
AssignmentExpression: 'Multiple CommonJS exports; consolidate all exports into a single assignment to `module.exports`',
}
/* eslint-enable max-len */

/**
* Returns an array with names of the properties in the accessor chain for MemberExpression nodes
*
* Example:
*
* `module.exports = {}` => ['module', 'exports']
* `module.exports.property = true` => ['module', 'exports', 'property']
*
* @param {Node} node AST Node (MemberExpression)
* @return {Array} Array with the property names in the chain
* @private
*/
function accessorChain(node) {
const chain = []

do {
chain.unshift(node.property.name)

if (node.object.type === 'Identifier') {
chain.unshift(node.object.name)
break
}

node = node.object
} while (node.type === 'MemberExpression')

return chain
}

function create(context) {
const nodes = {
modules: new Set(),
commonjs: new Set(),
}

return {
ExportNamedDeclaration(node) {
nodes.modules.add(node)
},

AssignmentExpression(node) {
if (node.left.type !== 'MemberExpression') {
return
}

const chain = accessorChain(node.left)

// Assignments to module.exports
// Deeper assignments are ignored since they just modify what's already being exported
// (ie. module.exports.exported.prop = true is ignored)
if (chain[0] === 'module' && chain[1] === 'exports' && chain.length <= 3) {
nodes.commonjs.add(node)
return
}

// Assignments to exports (exports.* = *)
if (chain[0] === 'exports' && chain.length === 2) {
nodes.commonjs.add(node)
return
}
},

'Program:exit': function onExit() {
// Report multiple `export` declarations (ES2015 modules)
if (nodes.modules.size > 1) {
nodes.modules.forEach(node => {
context.report({
node,
message: errors[node.type],
})
})
}

// Report multiple `module.exports` assignments (CommonJS)
if (nodes.commonjs.size > 1) {
nodes.commonjs.forEach(node => {
context.report({
node,
message: errors[node.type],
})
})
}
},
}
}

export default {
meta,
create,
}

0 comments on commit eca7b4d

Please sign in to comment.