[Compiler Bug] Complier mark ts instantiation expression as reorderable in build hir #34488
+81
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
...ugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.expect.md
Outdated
Show resolved
Hide resolved
...abel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.js
Outdated
Show resolved
Hide resolved
Updated the fixture to be a component and implemented id as a real function. Regenerated the snapshot. Eval output now renders <div>hi</div>.
|
@josephsavona Made the requested changes. PR is ready for another review |
…eorderable-in-BuildHIR
…eorderable-in-BuildHIR
|
@josephsavona I updated the branch to satisfy the up to date requirement. CI is green. Could you please re approve when you have a moment Thanks |
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Outdated
Show resolved
Hide resolved
josephsavona
requested changes
Sep 15, 2025
…tantiationExpression-as-reorderable-in-BuildHIR
…in-BuildHIR' of https://github.com/zekariasasaminew/react into compiler-mark-TSInstantiationExpression-as-reorderable-in-BuildHIR
use id<string> as default value and call inside the body
|
@josephsavona Thanks for the guidance. I updated isReorderableExpression to recurse for TSInstantiationExpression and switched the fixture to default to id as a value with the call inside the body. Snapshot regenerated and the filtered test passes locally. Ready for another look. |
…tantiationExpression-as-reorderable-in-BuildHIR
zekariasasaminew
force-pushed
the
compiler-mark-TSInstantiationExpression-as-reorderable-in-BuildHIR
branch
from
d095ca7 to
1db579e
Compare
…tantiationExpression-as-reorderable-in-BuildHIR
|
This should be ready @josephsavona |
…eorderable-in-BuildHIR
…eorderable-in-BuildHIR
|
Please stop pushing, every time you do it forces me to re-approve the workflow to make sure CI is green. |
github-actions bot
pushed a commit
that referenced
this pull request
Sep 18, 2025
…le in build hir (#34488) <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> The React Compiler rejected a default parameter that contains a TSInstantiationExpression with the todo message that the expression cannot be safely reordered. This change teaches the reorder check in BuildHIR.ts to treat TSInstantiationExpression as reorderable. This is safe because TypeScript instantiation only affects types and is erased at runtime, so it has no side effects and does not change semantics. ## How did you test this change? ``` Set-Content testfilter.txt 'ts-instantiation-default-param' yarn test --filter --update yarn test --filter ``` <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> I added a fixture: > compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.js DiffTrain build for [5813211](5813211)
github-actions bot
pushed a commit
that referenced
this pull request
Sep 18, 2025
…le in build hir (#34488) <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> The React Compiler rejected a default parameter that contains a TSInstantiationExpression with the todo message that the expression cannot be safely reordered. This change teaches the reorder check in BuildHIR.ts to treat TSInstantiationExpression as reorderable. This is safe because TypeScript instantiation only affects types and is erased at runtime, so it has no side effects and does not change semantics. ## How did you test this change? ``` Set-Content testfilter.txt 'ts-instantiation-default-param' yarn test --filter --update yarn test --filter ``` <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> I added a fixture: > compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.js DiffTrain build for [5813211](5813211)
This was referenced Sep 19, 2025
unstubbable
pushed a commit
to vercel/next.js
that referenced
this pull request
Sep 19, 2025
[diff facebook/react@84af9085...d415fd3e](facebook/react@84af908...d415fd3) <details> <summary>React upstream changes</summary> - facebook/react#34536 - facebook/react#34534 - facebook/react#34531 - facebook/react#34499 - facebook/react#34523 - facebook/react#34511 - facebook/react#34516 - facebook/react#34455 - facebook/react#34488 - facebook/react#34521 </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
The React Compiler rejected a default parameter that contains a TSInstantiationExpression with the todo message that the expression cannot be safely reordered. This change teaches the reorder check in BuildHIR.ts to treat TSInstantiationExpression as reorderable. This is safe because TypeScript instantiation only affects types and is erased at runtime, so it has no side effects and does not change semantics.How did you test this change?