-
-
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
New: group-exports
rule
#721
Merged
+413
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
doesn't it only warn when this is true and when they're not grouped together lexically?
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.
They do not need to be grouped lexically (I assume that means that the statements are consecutive), there must really be only a single assignment or export declaration.
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.
ah ok, thanks for clarifying.
In that case, is there a reason this rule couldn't be autofixable? (ie, delete all but the last export statements, and replace the last one with the combined group)
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 do not currently see any issues with auto-fixing, either, but I have not included a fixer in this PR for several reasons:
So I'd rather not add fixer at this moment, even though it should be possible to implement it, eventually.