Skip to content

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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