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 commonjs exports with destructuring. #5469
Changes from 9 commits
f81d749
9b410be
0c2e3b1
b5bb89b
06f67e1
b608e28
45b4174
c82b084
18d6ba9
c42e027
7035401
ddfb6f2
f2f226b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,6 +129,10 @@ export default function () { | |
return { | ||
inherits: require("babel-plugin-transform-strict-mode"), | ||
|
||
manipulateOptions(opts, parserOpts) { | ||
parserOpts.plugins.push("objectRestSpread"); | ||
}, | ||
|
||
visitor: { | ||
ThisExpression(path, state) { | ||
// If other plugins run after this plugin's Program#exit handler, we allow them to | ||
|
@@ -300,15 +304,42 @@ export default function () { | |
const id = decl.get("id"); | ||
|
||
const init = decl.get("init"); | ||
const exportsToInsert = []; | ||
if (!init.node) init.replaceWith(t.identifier("undefined")); | ||
|
||
if (id.isIdentifier()) { | ||
addTo(exports, id.node.name, id.node); | ||
init.replaceWith(buildExportsAssignment(id.node, init.node).expression); | ||
nonHoistedExportNames[id.node.name] = true; | ||
} else { | ||
// todo | ||
} else if (id.isObjectPattern()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we be handling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @loganfsmyth Sure! Going to add. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @xtuc I've started work on it but could pick it out to a separate PR |
||
for (let i = 0; i < id.node.properties.length; i++) { | ||
const prop = id.node.properties[i]; | ||
let propValue = prop.value; | ||
if (t.isAssignmentPattern(propValue)) { | ||
propValue = propValue.left; | ||
} else if (t.isRestProperty(prop)) { | ||
propValue = prop.argument; | ||
} | ||
addTo(exports, propValue.name, propValue); | ||
exportsToInsert.push(buildExportsAssignment(propValue, propValue)); | ||
nonHoistedExportNames[propValue.name] = true; | ||
} | ||
} else if (id.isArrayPattern() && id.node.elements) { | ||
for (let i = 0; i < id.node.elements.length; i++) { | ||
let elem = id.node.elements[i]; | ||
if (!elem) continue; | ||
if (t.isAssignmentPattern(elem)) { | ||
elem = elem.left; | ||
} else if (t.isRestElement(elem)) { | ||
elem = elem.argument; | ||
} | ||
const name = elem.name; | ||
addTo(exports, name, elem); | ||
exportsToInsert.push(buildExportsAssignment(elem, elem)); | ||
nonHoistedExportNames[name] = true; | ||
} | ||
} | ||
path.insertAfter(exportsToInsert); | ||
} | ||
path.replaceWith(declaration.node); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const [foo, bar = 2] = []; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
"use strict"; | ||
|
||
const [foo, bar = 2] = []; | ||
exports.foo = foo; | ||
exports.bar = bar; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const [foo, bar, ...baz] = []; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
"use strict"; | ||
|
||
const [foo, bar, ...baz] = []; | ||
exports.foo = foo; | ||
exports.bar = bar; | ||
exports.baz = baz; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const [foo, bar] = []; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
"use strict"; | ||
|
||
const [foo, bar] = []; | ||
exports.foo = foo; | ||
exports.bar = bar; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const { foo, bar = 1 } = {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
"use strict"; | ||
|
||
const { foo, bar = 1 } = {}; | ||
exports.foo = foo; | ||
exports.bar = bar; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const { foo, ...bar } = {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
"use strict"; | ||
|
||
const { foo, ...bar } = {}; | ||
exports.foo = foo; | ||
exports.bar = bar; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const { foo: bar, baz } = {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
"use strict"; | ||
|
||
const { foo: bar, baz } = {}; | ||
exports.bar = bar; | ||
exports.baz = baz; |
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.
Would be better to inherits from
babel-plugin-syntax-object-rest-spread
. It seems multiple inheritance is not possible (at least didn't found an example).To avoid that duplicated code, we could merge the config with the syntax plugin. What do you think?
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.
@xtuc Yes! Unfortunately, I didn't find multiple inheritance too. Can you please point me if there util for merging configs?
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.
It seems there are no merging util for configuration. I was thinking to use
Object.assign
.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.
Yes, just added
Object.assign
withbabel-plugin-transform-object-assign
as devDep to support node < 4.