Skip to content
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

[Fix] group-exports: Flow type export awareness #1707

Merged
merged 1 commit into from Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,
],
}),
],
})