facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.65k stars 47.02k forks source link

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

Open imjordanxd opened 2 weeks ago

imjordanxd commented 2 weeks ago

What kind of issue is this?

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

josephsavona commented 2 weeks ago

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.

stipsan commented 2 weeks ago

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

hernandemonteiro commented 1 week ago

From your description, this issue primarily falls under React Compiler Core since it seems related to how React's compiler is handling your lazy initializer function, specifically that it’s caching it unexpectedly in the compiled output. This behavior causes the function to persist unnecessarily, which could be an unintended compiler optimization issue impacting the functionality of the component.

Key Points from Your Issue: