X Tutup
Skip to content

Allow mixing private destructuring and rest#17534

Merged
nicolo-ribaudo merged 9 commits intobabel:mainfrom
CO0Ki3:feat/destructuring-private-split-explicit-and-rest
Oct 17, 2025
Merged

Allow mixing private destructuring and rest#17534
nicolo-ribaudo merged 9 commits intobabel:mainfrom
CO0Ki3:feat/destructuring-private-split-explicit-and-rest

Conversation

@CO0Ki3
Copy link
Contributor

@CO0Ki3 CO0Ki3 commented Oct 1, 2025

Q                       A
Fixed Issues? Fixes #17282
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

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)

@babel-bot
Copy link
Collaborator

babel-bot commented Oct 1, 2025

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/60059

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 1, 2025

Open in StackBlitz

commit: 6d39df4

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This PR now looks very good and I don't have more comments.

독고현 and others added 3 commits October 13, 2025 17:18
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

This was referenced Nov 27, 2025
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 17, 2026
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect output when destructuring private property with duplicate properties and rest element

4 participants

X Tutup