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 #328: properly suppress default when looking at * exports. #332

Merged
merged 2 commits into from
May 11, 2016
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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