Skip to content

Conversation

@josephsavona
Copy link
Member

@josephsavona josephsavona commented Nov 14, 2025

Destructing statements that start off as declarations can end up becoming reassignments if the variable is a scope declaration, so we have existing logic to handle cases where some parts of a destructure need to be converted into new locals, with a reassignment to the hoisted scope variable afterwards. However, there is an edge case where all of the values are reassigned, in which case we don't need to rewrite and can just set the instruction kind to reassign.


Stack created with Sapling. Best reviewed with ReviewStack.

@meta-cla meta-cla bot added the CLA Signed label Nov 14, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Nov 14, 2025
Comment on lines -1371 to -1390
const isDeclared = cx.hasDeclared(place.identifier);
hasReassign ||= isDeclared;
hasDeclaration ||= !isDeclared;
}
if (hasReassign && hasDeclaration) {
CompilerError.invariant(false, {
reason:
'Encountered a destructuring operation where some identifiers are already declared (reassignments) but others are not (declarations)',
description: null,
details: [
{
kind: 'error',
loc: instr.loc,
message: null,
},
],
suggestions: null,
});
} else if (hasReassign) {
kind = InstructionKind.Reassign;
Copy link
Member Author

Choose a reason for hiding this comment

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

the logic for hasDeclared() here doesn't account for all local variables, so it was incorrectly reporting that the fixed destructuring had mixed reassignment and declarations

josephsavona added a commit that referenced this pull request Nov 14, 2025
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35145).
* #35144
* __->__ #35145
Destructing statements that start off as declarations can end up becoming reassignments if the variable is a scope declaration, so we have existing logic to handle cases where some parts of a destructure need to be converted into new locals, with a reassignment to the hoisted scope variable afterwards. However, there is an edge case where all of the values are reassigned, in which case we don't need to rewrite and can just set the instruction kind to reassign.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants