Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Compiler Optimization]: avoid memoizing useState lazy initializer functions #31453

Open
1 of 4 tasks
imjordanxd opened this issue Nov 7, 2024 · 2 comments
Open
1 of 4 tasks

Comments

@imjordanxd
Copy link

imjordanxd commented Nov 7, 2024

What kind of issue is this?

  • React Compiler core (the JS output is incorrect, or your app works incorrectly after optimization)
  • babel-plugin-react-compiler (build issue installing or using the Babel plugin)
  • eslint-plugin-react-compiler (build issue installing or using the eslint plugin)
  • react-compiler-healthcheck (build issue installing or using the healthcheck script)

Link to repro

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEAYhBABQAOMEVYyRwmAhgLYLJg4x6YDmAXwCUTIgB1iROIW5EA2txY4EAXSIBeIgCUELXADooYBAGUcyhBQqiNAPiaSizojAQ5YxTAgDuRACosANYIYAAyhPz+eBz+EADCEGxUUCrUtPQGrBzCTkQiANx5bh4wxEoqWewIRZiCkpJwADYsYGABwaERAtGxCUkpKo5S2ZzcvAK1zjKY41C4EDAUo6LAec5oi0QUTe5EeJpEAAwF+wDUZ6cHADxEAIwIAGyr+etEOAAWeGBVHIejU1edRAgiAA

Repro steps

  1. call useState with the lazy state initialiser that consumes props
  2. observe the function is being cached in the compiled code despite it only ever being called once

However, I'm not 100% sure this is valid React code as the docs state:

If you pass a function as initialState, it will be treated as an initializer function. It should be pure, should take no arguments, and should return a value of any type. React will call your initializer function when initializing the component, and store its return value as the initial state. See an example below.

But this does seem like a valid use-case.

How often does this bug happen?

Every time

What version of React are you using?

version in playground

What version of React Compiler are you using?

version in playground

@imjordanxd imjordanxd added Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug labels Nov 7, 2024
@imjordanxd imjordanxd changed the title [Compiler Bug]: useState lazy initialiser function incorrectly cached [Compiler Bug]: useState lazy initialiser function incorrectly cached Nov 7, 2024
@josephsavona josephsavona removed Type: Bug Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Nov 8, 2024
@josephsavona josephsavona changed the title [Compiler Bug]: useState lazy initialiser function incorrectly cached [Compiler Bug]: useState lazy initialiser function unnecessarily cached Nov 8, 2024
@josephsavona
Copy link
Contributor

Thanks for posting. Note that React will only call the initializer function once. It's perfectly fine for the compiler to cache it, but you're right that it's not strictly necessary. The memoization here is fine from a correctness perspective and hasn't been a problem in practice so it is simply a potential optimization to avoid memoizing the value.

We have a pass that prunes memoization for non-escaping values, and we would approach this by teaching that pass about lazy state initializers.

@josephsavona josephsavona changed the title [Compiler Bug]: useState lazy initialiser function unnecessarily cached [Compiler Optimization]: avoid memoizing useState lazy initializer functions Nov 8, 2024
@stipsan
Copy link

stipsan commented Nov 8, 2024

I think there's a similar opportunity for useSyncExternalStore and getSnapshot + getServerSnapshot, as only subscribe needs to be memoized 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants