Skip to content

Commit

Permalink
fix #328: properly suppress default when looking at * exports. (#332)
Browse files Browse the repository at this point in the history
* fix #328: properly suppress `default` when looking at * exports.

* damnnnn, curly. back at it again with indented blocks
  • Loading branch information
benmosher committed May 11, 2016
1 parent e049817 commit 1e9c371
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 30 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]
### Fixed
- `export * from 'foo'` now properly ignores a `default` export from `foo`, if any. ([#328]/[#332], thanks [@jkimbo])
This impacts all static analysis of imported names. ([`default`], [`named`], [`namespace`], [`export`])

## [1.8.0] - 2016-05-11
### Added
- [`prefer-default-export`], new rule. ([#308], thanks [@gavriguy])
Expand Down Expand Up @@ -206,10 +211,13 @@ for info on changes for earlier releases.
[`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md
[`order`]: ./docs/rules/order.md
[`named`]: ./docs/rules/named.md
[`default`]: ./docs/rules/default.md
[`export`]: ./docs/rules/export.md
[`newline-after-import`]: ./docs/rules/newline-after-import.md
[`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md
[`prefer-default-export`]: ./docs/rules/prefer-default-export.md

[#332]: https://github.com/benmosher/eslint-plugin-import/pull/332
[#322]: https://github.com/benmosher/eslint-plugin-import/pull/322
[#316]: https://github.com/benmosher/eslint-plugin-import/pull/316
[#308]: https://github.com/benmosher/eslint-plugin-import/pull/308
Expand All @@ -235,6 +243,7 @@ for info on changes for earlier releases.
[#164]: https://github.com/benmosher/eslint-plugin-import/pull/164
[#157]: https://github.com/benmosher/eslint-plugin-import/pull/157

[#328]: https://github.com/benmosher/eslint-plugin-import/issues/328
[#317]: https://github.com/benmosher/eslint-plugin-import/issues/317
[#286]: https://github.com/benmosher/eslint-plugin-import/issues/286
[#281]: https://github.com/benmosher/eslint-plugin-import/issues/281
Expand Down Expand Up @@ -290,3 +299,4 @@ for info on changes for earlier releases.
[@josh]: https://github.com/josh
[@borisyankov]: https://github.com/borisyankov
[@gavriguy]: https://github.com/gavriguy
[@jkimbo]: https://github.com/jkimbo
58 changes: 34 additions & 24 deletions src/core/getExports.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,16 @@ export default class ExportMap {
if (this.namespace.has(name)) return true
if (this.reexports.has(name)) return true

for (let dep of this.dependencies.values()) {
let innerMap = dep()
// default exports must be explicitly re-exported (#328)
if (name !== 'default') {
for (let dep of this.dependencies.values()) {
let innerMap = dep()

// todo: report as unresolved?
if (!innerMap) continue
// todo: report as unresolved?
if (!innerMap) continue

if (innerMap.has(name)) return true
if (innerMap.has(name)) return true
}
}

return false
Expand Down Expand Up @@ -264,18 +267,22 @@ export default class ExportMap {
return deep
}

for (let dep of this.dependencies.values()) {
let innerMap = dep()
// todo: report as unresolved?
if (!innerMap) continue

// safeguard against cycles
if (innerMap.path === this.path) continue
// default exports must be explicitly re-exported (#328)
if (name !== 'default') {
for (let dep of this.dependencies.values()) {
let innerMap = dep()
// todo: report as unresolved?
if (!innerMap) continue

// safeguard against cycles
if (innerMap.path === this.path) continue

let innerValue = innerMap.hasDeep(name)
if (innerValue.found) {
innerValue.path.unshift(this)
return innerValue
let innerValue = innerMap.hasDeep(name)
if (innerValue.found) {
innerValue.path.unshift(this)
return innerValue
}
}
}

Expand All @@ -298,16 +305,19 @@ export default class ExportMap {
return imported.get(local)
}

for (let dep of this.dependencies.values()) {
let innerMap = dep()
// todo: report as unresolved?
if (!innerMap) continue
// default exports must be explicitly re-exported (#328)
if (name !== 'default') {
for (let dep of this.dependencies.values()) {
let innerMap = dep()
// todo: report as unresolved?
if (!innerMap) continue

// safeguard against cycles
if (innerMap.path === this.path) continue
// safeguard against cycles
if (innerMap.path === this.path) continue

let innerValue = innerMap.get(name)
if (innerValue !== undefined) return innerValue
let innerValue = innerMap.get(name)
if (innerValue !== undefined) return innerValue
}
}

return undefined
Expand All @@ -321,7 +331,7 @@ export default class ExportMap {
callback.call(thisArg, getImport().get(local), name, this))

this.dependencies.forEach(dep => dep().forEach((v, n) =>
callback.call(thisArg, v, n, this)))
n !== 'default' && callback.call(thisArg, v, n, this)))
}

// todo: keys, values, entries?
Expand Down
5 changes: 4 additions & 1 deletion src/rules/export.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ module.exports = function (context) {
return
}
let any = false
remoteExports.forEach((v, name) => (any = true) && addNamed(name, node))
remoteExports.forEach((v, name) =>
name !== 'default' &&
(any = true) && // poor man's filter
addNamed(name, node))

if (!any) {
context.report(node.source,
Expand Down
1 change: 1 addition & 0 deletions tests/files/deep-es7/b.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * as c from './c'
export default 'b'
3 changes: 2 additions & 1 deletion tests/files/deep/b.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import * as c from './c'
export { c }
export { c }
export default 'b'
3 changes: 3 additions & 0 deletions tests/files/re-export.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export const c = 'foo'

export * from './named-exports'

// #328: this exports only 'foo', not the default.
export * from './bar'
6 changes: 6 additions & 0 deletions tests/src/rules/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,11 @@ ruleTester.run('default', rule, {
parser: 'babel-eslint',
errors: ['No default export found in module.'],
}),

// #328: * exports do not include default
test({
code: 'import barDefault from "./re-export"',
errors: [`No default export found in module.`],
}),
],
})
13 changes: 9 additions & 4 deletions tests/src/rules/export.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ ruleTester.run('export', rule, {
test({ code: 'export { bar }; export * from "./export-all"' }),
test({ code: 'export * from "./export-all"' }),
test({ code: 'export * from "./does-not-exist"' }),

// #328: "export * from" does not export a default
test({ code: 'export default foo; export * from "./bar"' }),
],

invalid: [
Expand All @@ -29,10 +32,6 @@ ruleTester.run('export', rule, {
code: 'export default foo; export default bar',
errors: ['Multiple default exports.', 'Multiple default exports.'],
}),
test({
code: 'export default foo; export * from "./default-export"',
errors: ['Multiple default exports.', 'Multiple default exports.'],
}),
test({
code: 'export default function foo() {}; ' +
'export default function bar() {}',
Expand Down Expand Up @@ -99,5 +98,11 @@ ruleTester.run('export', rule, {
'Multiple exports of name \'bar\'.'],
}),


// #328: "export * from" does not export a default
test({
code: 'export * from "./default-export"',
errors: [`No named exports found in module './default-export'.`],
}),
],
})
6 changes: 6 additions & 0 deletions tests/src/rules/named.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,5 +217,11 @@ ruleTester.run('named', rule, {
// todo: better error message
errors: ["common not found via re-export-default.js -> common.js"],
}),

// #328: * exports do not include default
test({
code: 'import { default as barDefault } from "./re-export"',
errors: [`default not found in './re-export'`],
}),
],
})
10 changes: 10 additions & 0 deletions tests/src/rules/namespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const valid = [

// names.default is valid export
test({ code: "import * as names from './default-export';" }),
test({ code: "import * as names from './default-export'; console.log(names.default)" }),
test({
code: 'export * as names from "./default-export"',
parser: 'babel-eslint',
Expand Down Expand Up @@ -164,6 +165,12 @@ const invalid = [
errors: [error('c', 'names')],
}),

// #328: * exports do not include default
test({
code: 'import * as ree from "./re-export"; console.log(ree.default)',
errors: [`'default' not found in imported namespace 'ree'.`],
}),

]

///////////////////////
Expand All @@ -177,6 +184,9 @@ const invalid = [
test({ parser, code: `import * as a from "./${folder}/a"; var {b:{c:{d:{e}}}} = a` }),
test({ parser, code: `import { b } from "./${folder}/a"; var {c:{d:{e}}} = b` }))

// deep namespaces should include explicitly exported defaults
test({ parser, code: `import * as a from "./${folder}/a"; console.log(a.b.default)` }),

invalid.push(
test({
parser,
Expand Down

0 comments on commit 1e9c371

Please sign in to comment.