Skip to content

Conversation

@poteto
Copy link
Member

@poteto poteto commented Mar 12, 2025

Not sure when this happened but the playground tests have gotten a little bit flaky. I suspect it's because of timeouts since it happens randomly so let's see if this helps.

Stack created with Sapling. Best reviewed with ReviewStack.

Extracting portions of #32416 for easier review. This PR dedupes @babel/types to resolve to 7.26.3, for compatibility in the root workspace where eslint-plugin-react-hooks resides.

I also needed to update @babel/preset-typescript in snap.

The compiler changes in HIR and ReactiveScopes were needed due to types changing. Notably, Babel [added support for optional chaining assignment](babel/babel#15751) (currently [Stage 1](https://github.com/tc39/proposal-optional-chaining-assignment)), so in the latest versions of @babel/types, AssignmentExpression.left can now also be of t.OptionalMemberExpression.

Given that this is in Stage 1, the compiler probably shouldn't support this syntax, so this PR updates HIR to bailout with a TODO if there is a non LVal on the lhs of an Assignment Expression.

There was also a small superficial SourceLocation change needed in `InferReactiveScopeVariables` as Babel 8 changes were [accidentally released in 7](babel/babel#10746 (comment)). It doesn't affect our analysis so it seems fine to just update with the new properties.

Co-authored-by: michael faith <michaelfaith@users.noreply.github.com>
Not sure when this happened but the playground tests have gotten a little bit flaky. I suspect it's because of timeouts since it happens randomly so let's see if this helps.
Copy link
Contributor

@mofeiZ mofeiZ left a comment

Choose a reason for hiding this comment

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

Thanks for trying to fix ci! I noticed that playground tests are pretty flakey as well, although runlogs I've seen show that spurious failures are due to some static fonts (e.g. fetching from localhost:3000/fonts/Optimistic_Display_W_Md.woff2 returning a 404)

  1. I visited Actions -> Compiler: Playground -> Action details page e.g. https://github.com/facebook/react/actions/runs/13808144588
  2. downloaded and unzipped runtime artifacts, then ran yarn playwright show-trace

I've tried reproing locally but it was pretty flakey 😅

Huh actually, I just noticed that this file isn't checked into source control while other font files are.. That looks surprising

Update:
Ohh I wonder if this is related to these fonts being in the gitignore and downloaded dynamically. That could explain why I haven't seen a 404 for Source-Code-Pro-Regular.woff2

@poteto
Copy link
Member Author

poteto commented Mar 12, 2025

@mofeiZ I've tried that before too but I don't think it's the missing font files causing the error. If you look at the screenshots included in test-results.zip (in the CI build artifact) you'll see that the failing ones only show the header and everything else is blank

@poteto
Copy link
Member Author

poteto commented Mar 12, 2025

Gonna abandon this one, doesn't seem to help

@poteto poteto closed this Mar 12, 2025
@poteto poteto deleted the pr32582 branch March 12, 2025 21:01
poteto added a commit that referenced this pull request Mar 12, 2025
Extracting portions of #32416 for easier review. This PR dedupes
@babel/types to resolve to 7.26.3, for compatibility in the root
workspace where eslint-plugin-react-hooks resides.

I also needed to update @babel/preset-typescript in snap.

The compiler changes in HIR and ReactiveScopes were needed due to types
changing. Notably, Babel [added support for optional chaining
assignment](babel/babel#15751) (currently [Stage
1](https://github.com/tc39/proposal-optional-chaining-assignment)), so
in the latest versions of @babel/types, AssignmentExpression.left can
now also be of t.OptionalMemberExpression.

Given that this is in Stage 1, the compiler probably shouldn't support
this syntax, so this PR updates HIR to bailout with a TODO if there is a
non LVal on the lhs of an Assignment Expression.

There was also a small superficial SourceLocation change needed in
`InferReactiveScopeVariables` as Babel 8 changes were [accidentally
released in
7](babel/babel#10746 (comment)).
It doesn't affect our analysis so it seems fine to just update with the
new properties.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32581).
* #32582
* __->__ #32581

Co-authored-by: michael faith <michaelfaith@users.noreply.github.com>

Co-authored-by: michael faith <michaelfaith@users.noreply.github.com>
@mofeiZ
Copy link
Contributor

mofeiZ commented Mar 12, 2025

Ahh gotcha, makes sense. Thanks for explaining!

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.

4 participants