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 3 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 |
---|---|---|
|
@@ -306,6 +306,15 @@ export default function () { | |
addTo(exports, id.node.name, id.node); | ||
init.replaceWith(buildExportsAssignment(id.node, init.node).expression); | ||
nonHoistedExportNames[id.node.name] = true; | ||
} else if (id.isObjectPattern()) { | ||
for (let i = 0; i < id.node.properties.length; i++) { | ||
const prop = id.node.properties[i]; | ||
if (!t.isRestProperty(prop)) { | ||
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. Shouldn't the rest property also be handled? for example
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. 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. It's fine for people to have enabled support for the syntax without enabling the actual transformation, so I think it would be good to handle. |
||
addTo(exports, prop.value.name, prop.value); | ||
path.insertAfter(buildExportsAssignment(prop.value, prop.value)); | ||
nonHoistedExportNames[prop.value.name] = true; | ||
} | ||
} | ||
} else { | ||
// todo | ||
} | ||
|
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.baz = baz; | ||
exports.bar = bar; |
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.
Should we be handling
ArrayPattern
too?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.
@loganfsmyth Sure! Going to add. Thanks!
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.
Could the
ArrayPattern
be a separate PR?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 I've started work on it but could pick it out to a separate PR