loosen computed key restriction for compiler #3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Remove restriction on complex expressions as computed property keys in object literals. The restriction was added to prevent a mutation tracking bug in the original mutation aliasing model, but the new mutation aliasing model (now the default) correctly handles these cases.
Changes
CallExpressionand other complex expressions as computed property keyslowerExpressionToTemporaryfunction already handles all expression types correctly (CallExpression,SequenceExpression, etc.)Converted 4
error.todo-tests to passing tests:object-expression-member-expr-call.js- CallExpression via MemberExpressionobject-expression-computed-key-modified-during-after-construction.js- Direct CallExpressionobject-expression-computed-key-mutate-key-while-constructing-object.js- CallExpression during constructionobject-expression-computed-key-modified-during-after-construction-sequence-expr.js- SequenceExpressionBackground
The restriction was added to avoid issues where mutations were not tracked accurately/consistently. Example of the problematic pattern was visible in this bug (recently removed as part of the new mutation aliasing model rollout): https://github.com/facebook/react/blob/d58c07b563a79bd706531278cb4afec6292b84a8/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md
How the New Model Fixes This
The new mutation aliasing model (documented in
MUTABILITY_ALIASING_MODEL.md) builds a data flow graph to track mutations and aliasing. From the documentation:When processing the computed key
(mutate(key), key), the compiler lowers it to a temporary value. The model then infers anAssigneffect from the sequence expression result to this temporary.Two key rules from the mutation model apply. First, "Mutation of Assignment Mutates the Source Value":
And second, "Mutation of Source Affects Alias, Assignment, CreateFrom, and Capture":
As the documentation explains: "A derived value changes when its source value is mutated."
In our case, the computed key expression creates an assignment relationship between the temporary and
key. Whenkeyis mutated later, this mutation propagates to the temporary through the assignment edge in the data flow graph. The mutable range of the temporary extends to include all mutations ofkey, preventing incorrect separate memoization of the object construction.Follow-up
Note: Many fixture files still contain the
@enableNewMutationAliasingModelpragma. This pragma is now obsolete (the new model is the default and the config option has been removed). Cleaning up these pragmas would be a good follow-up task. The presence of this pragma was initially confusing during investigation—it required checking git history to understand that it had been cleaned up and is now the default behavior.