Skip to content

Commit

Permalink
[Fix] group-exports: Flow type export awareness
Browse files Browse the repository at this point in the history
Fixes #1702.
  • Loading branch information
ernestostifano authored and ljharb committed Mar 29, 2020
1 parent 6a110dd commit baf1a8c
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 15 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## [Unreleased]

### Fixed
- [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano])

## [2.20.2] - 2020-03-28
### Fixed
- [`order`]: fix `isExternalModule` detect on windows ([#1651], thanks [@fisker])
Expand Down Expand Up @@ -660,6 +663,7 @@ for info on changes for earlier releases.

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

[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702
[#1666]: https://github.com/benmosher/eslint-plugin-import/pull/1666
[#1664]: https://github.com/benmosher/eslint-plugin-import/pull/1664
[#1658]: https://github.com/benmosher/eslint-plugin-import/pull/1658
Expand Down Expand Up @@ -988,6 +992,7 @@ for info on changes for earlier releases.
[0.12.1]: https://github.com/benmosher/eslint-plugin-import/compare/v0.12.0...v0.12.1
[0.12.0]: https://github.com/benmosher/eslint-plugin-import/compare/v0.11.0...v0.12.0
[0.11.0]: https://github.com/benmosher/eslint-plugin-import/compare/v0.10.1...v0.11.0

[@mathieudutour]: https://github.com/mathieudutour
[@gausie]: https://github.com/gausie
[@singles]: https://github.com/singles
Expand Down Expand Up @@ -1122,3 +1127,4 @@ for info on changes for earlier releases.
[@fisker]: https://github.com/fisker
[@richardxia]: https://github.com/richardxia
[@TheCrueltySage]: https://github.com/TheCrueltySage
[@ernestostifano]: https://github.com/ernestostifano
18 changes: 18 additions & 0 deletions docs/rules/group-exports.md
Expand Up @@ -60,6 +60,15 @@ test.another = true
module.exports = test
```

```flow js
const first = true;
type firstType = boolean
// A single named export declaration (type exports handled separately) -> ok
export {first}
export type {firstType}
```


### Invalid

Expand Down Expand Up @@ -94,6 +103,15 @@ module.exports.first = true
module.exports.second = true
```

```flow js
type firstType = boolean
type secondType = any
// Multiple named type export statements -> not ok!
export type {firstType}
export type {secondType}
```

## 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 package.json
Expand Up @@ -62,6 +62,7 @@
"babel-plugin-istanbul": "^4.1.6",
"babel-plugin-module-resolver": "^2.7.1",
"babel-preset-es2015-argon": "latest",
"babel-preset-flow": "^6.23.0",
"babel-register": "^6.26.0",
"babylon": "^6.18.0",
"chai": "^4.2.0",
Expand Down
57 changes: 43 additions & 14 deletions src/rules/group-exports.js
Expand Up @@ -46,19 +46,28 @@ function accessorChain(node) {

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

return {
ExportNamedDeclaration(node) {
let target = node.exportKind === 'type' ? nodes.types : nodes.modules
if (!node.source) {
nodes.modules.add(node)
} else if (Array.isArray(nodes.sources[node.source.value])) {
nodes.sources[node.source.value].push(node)
target.set.add(node)
} else if (Array.isArray(target.sources[node.source.value])) {
target.sources[node.source.value].push(node)
} else {
nodes.sources[node.source.value] = [node]
target.sources[node.source.value] = [node]
}
},

Expand All @@ -73,21 +82,21 @@ function create(context) {
// 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)
nodes.commonjs.set.add(node)
return
}

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

'Program:exit': function onExit() {
// Report multiple `export` declarations (ES2015 modules)
if (nodes.modules.size > 1) {
nodes.modules.forEach(node => {
if (nodes.modules.set.size > 1) {
nodes.modules.set.forEach(node => {
context.report({
node,
message: errors[node.type],
Expand All @@ -96,7 +105,27 @@ function create(context) {
}

// Report multiple `aggregated exports` from the same module (ES2015 modules)
flat(values(nodes.sources)
flat(values(nodes.modules.sources)
.filter(nodesWithSource => Array.isArray(nodesWithSource) && nodesWithSource.length > 1))
.forEach((node) => {
context.report({
node,
message: errors[node.type],
})
})

// Report multiple `export type` declarations (FLOW ES2015 modules)
if (nodes.types.set.size > 1) {
nodes.types.set.forEach(node => {
context.report({
node,
message: errors[node.type],
})
})
}

// Report multiple `aggregated type exports` from the same module (FLOW ES2015 modules)
flat(values(nodes.types.sources)
.filter(nodesWithSource => Array.isArray(nodesWithSource) && nodesWithSource.length > 1))
.forEach((node) => {
context.report({
Expand All @@ -106,8 +135,8 @@ function create(context) {
})

// Report multiple `module.exports` assignments (CommonJS)
if (nodes.commonjs.size > 1) {
nodes.commonjs.forEach(node => {
if (nodes.commonjs.set.size > 1) {
nodes.commonjs.set.forEach(node => {
context.report({
node,
message: errors[node.type],
Expand Down
62 changes: 61 additions & 1 deletion tests/src/rules/group-exports.js
@@ -1,14 +1,25 @@
import { test } from '../utils'
import { RuleTester } from 'eslint'
import rule from 'rules/group-exports'
import {resolve} from 'path'
import {default as babelPresetFlow} from 'babel-preset-flow'

/* eslint-disable max-len */
const errors = {
named: 'Multiple named export declarations; consolidate all named exports into a single export declaration',
commonjs: 'Multiple CommonJS exports; consolidate all exports into a single assignment to `module.exports`',
}
/* eslint-enable max-len */
const ruleTester = new RuleTester()
const ruleTester = new RuleTester({
parser: resolve(__dirname, '../../../node_modules/babel-eslint'),
parserOptions: {
babelOptions: {
configFile: false,
babelrc: false,
presets: [babelPresetFlow],
},
},
})

ruleTester.run('group-exports', rule, {
valid: [
Expand Down Expand Up @@ -103,6 +114,27 @@ ruleTester.run('group-exports', rule, {
unrelated = 'assignment'
module.exports.test = true
` }),
test({ code: `
type firstType = {
propType: string
};
const first = {};
export type { firstType };
export { first };
` }),
test({ code: `
type firstType = {
propType: string
};
type secondType = {
propType: string
};
export type { firstType, secondType };
` }),
test({ code: `
export type { type1A, type1B } from './module-1'
export { method1 } from './module-1'
` }),
],
invalid: [
test({
Expand Down Expand Up @@ -231,5 +263,33 @@ ruleTester.run('group-exports', rule, {
errors.commonjs,
],
}),
test({
code: `
type firstType = {
propType: string
};
type secondType = {
propType: string
};
const first = {};
export type { firstType };
export type { secondType };
export { first };
`,
errors: [
errors.named,
errors.named,
],
}),
test({
code: `
export type { type1 } from './module-1'
export type { type2 } from './module-1'
`,
errors: [
errors.named,
errors.named,
],
}),
],
})

0 comments on commit baf1a8c

Please sign in to comment.