Allow mixing private destructuring and rest #17534
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/60059 |
|
commit: |
JLHwung
left a comment
JLHwung
left a comment
There was a problem hiding this comment.
Thank you for your contribution. This PR looks good to me generally, pending literal checks in the duplicate keys detection.
| propertyKey.type === "Identifier" | ||
| ) { | ||
| return existing.key.name === propertyKey.name; | ||
| } |
There was a problem hiding this comment.
Nit: A literal property name can also be one of
- string literal (
"0") - numerical literal (
0) - bigint literal (
0n)
To optimize the output, we should also check these AST types for potential duplicates.
There was a problem hiding this comment.
@JLHwung
Thanks for review.
I've added the checks for the types you mentioned.
Are there any other types or edge cases I should consider?
JLHwung
left a comment
JLHwung
left a comment
There was a problem hiding this comment.
Thank you! This PR now looks very good and I don't have more comments.
remark: add tc39 link Co-authored-by: Huáng Jùnliàng <[email protected]>
nicolo-ribaudo
changed the title
nicolo-ribaudo
added
the
PR: Bug Fix 🐛
4 participants
Summary
This PR fixes a bug in @babel/plugin-proposal-destructuring-private where explicit properties after a private key were destructured from an already-excluded object, causing values to become undefined.
What changed
Split post-private segment into two declarators:
bind explicit props (e.g.
{ a: x };In Issue example) from the original RHS (C)bind only the rest identifier from objectWithoutProperties(RHS, excluded)
Grow excludingKeys before emitting explicit props so rest properly excludes them.
Memoize non-static computed keys during excludingKeys growth (e.g. [_m = a()]) so the same key is reused by both explicit and rest parts (single evaluation).
Dedupe non-computed keys (Identifier, StringLiteral, NumericLiteral, BigIntLiteral)