Skip to content

Conversation

@everettbu
Copy link

@everettbu everettbu commented Dec 13, 2025

Mirror of facebook/react#35298
Original author: josephsavona


We currently model impure functions like Date.now() with an Impure effect, but the way this works is that the effect is considered an error if it's called at all. But we really want to reflect the fact that the value is impure, and is okay to be used outside of render. For example, useRef(Date.now()) should be fine.

So this PR changes the Impure effect to describe that an impure value flows into: Place. We flow impurity through the data flow graph during InferMutationAliasingRanges, including propagating this to the function's effects. So if a function expression returns a transitively impure value, that's expressed in our inferred signature for that function. Calling it propagates the impurity via our effect system.

We stop this propagation when reaching a ref, allowing useRef(Date.now()) or useRef(localFunctionThatReturnsDateNow()).

This lets us also model accessing a ref as an impure value - we just emit an Impure event for PropertyLoad/ComputedLoad off of a ref, and the above tracking kicks in.

A final piece is that we can also use our existing machinery to disallow writing to global values during render to handle writing to refs - except that we need to allow the lazy init pattern. So it probably makes sense to keep the ref validation pass, but scope it back to just handling writes and not reads.

This definitely needs more polish, the error messages would ideally be something like:

Error: cannot call impure function in render

Blah blah description of why this is bad...

foo.js:0:0
  <Foo x={x} />
          ^ This value reads from an impure function

foo.js:0:0
  x = Date.now();
      ^^^^^^^^ The value derives from Date.now which is impure

Ie show you both the local site (where impurity flows into render) and the ultiamte source of the impure value.

We currently model impure functions like `Date.now()` with an `Impure` effect, but the way this works is that the effect is considered an error if it's called at all. But we really want to reflect the fact that the _value_ is impure, and is okay to be used outside of render. For example, `useRef(Date.now())` should be fine.

So this PR changes the Impure effect to describe that an impure value flows `into: Place`. We flow impurity through the data flow graph during InferMutationAliasingRanges, including propagating this to the function's effects. So if a function expression returns a transitively impure value, that's expressed in our inferred signature for that function. Calling it propagates the impurity via our effect system.

We stop this propagation when reaching a ref, allowing `useRef(Date.now())` or `useRef(localFunctionThatReturnsDateNow())`.

This lets us also model accessing a ref as an impure value - we just emit an `Impure` event for PropertyLoad/ComputedLoad off of a ref, and the above tracking kicks in.

A final piece is that we can also use our existing machinery to disallow writing to global values during render to handle writing to refs - except that we need to allow the lazy init pattern. So it probably makes sense to keep the ref validation pass, but scope it back to just handling writes and not reads.

This definitely needs more polish, the error messages would ideally be something like:

```
Error: cannot call impure function in render

Blah blah description of why this is bad...

foo.js:0:0
  <Foo x={x} />
          ^ This value reads from an impure function

foo.js:0:0
  x = Date.now();
      ^^^^^^^^ The value derives from Date.now which is impure
```

Ie show you both the local site (where impurity flows into render) and the ultiamte source of the impure value.
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Dec 13, 2025
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.

3 participants