Skip to content

Conversation

note: This implements the idea discussed in reactwg/react#389 (comment)

Currently we create `Impure` effects for impure functions like `Date.now()` or `Math.random()`, and then throw if the effect is reachable during render. However, impurity is a property of the resulting value: if the value isn't accessed during render then it's okay: maybe you're console-logging the time while debugging (fine), or storing the impure value into a ref and only accessing it in an effect or event handler (totally ok).

This PR updates to validate that impure values are not transitively consumed during render, building on the `Render` effect. The validation currently uses an algorithm very similar to that of InferReactivePlaces - building a set of known impure values, starting with `Impure` effects as the sources and then flowing outward via data flow and mutations. If a transitively impure value reaches a `Render` effect, it's an error. We record both the source of the impure value and where it gets rendered to make it easier to understand and fix errors:

```
Error: Cannot access impure value during render

Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent).

error.invalid-impure-functions-in-render-via-render-helper.ts:10:25
   8 |     const array = makeArray(now);
   9 |     const hasDate = identity(array);
> 10 |     return <Bar hasDate={hasDate} />;
     |                          ^^^^^^^ Cannot access impure value during render
  11 |   };
  12 |   return <Foo renderItem={renderItem} />;
  13 | }

error.invalid-impure-functions-in-render-via-render-helper.ts:6:14
  4 |
  5 | function Component() {
> 6 |   const now = Date.now();
    |               ^^^^^^^^^^ `Date.now` is an impure function.
  7 |   const renderItem = () => {
  8 |     const array = makeArray(now);
  9 |     const hasDate = identity(array);
```

Impure values are allowed to flow into refs, meaning that we now allow `useRef(Date.now())` or `useRef(localFunctionThatReturnsMathDotRandom())` which would have errored previously. The next PR reuses this improved impurity tracking to validate ref access in render as well.
…idation

Updates to guard against *reading* refs during render via the improved validateNoImpureValuesInRender() pass. InferMutationAliasingEffects generates `Impure` effects for ref reads, and then this pass validates that those accesses don't flow into `Render` effects. We now call the impure value validation first so that it takes priority over validateNoRefAccessInRender - the latter still has all the existing logic for now, but we can dramatically simplify it in a follow-up PR so that it only has to validate against ref writes.
Rewrites ValidateNoRefAccessInRender to use a simpler single-pass algorithm
that only validates ref mutations (reads are now handled separately by
ValidateNoImpureValuesInRender).

The new approach:
- Tracks refs and ref values through the function
- Identifies functions that mutate refs (directly or transitively)
- Only errors when ref-mutating functions are called at the top level
- Supports null-guard exception: mutations inside `if (ref.current == null)`
  are allowed for the initialization pattern

This reduces ~700 lines of complex fixpoint iteration to ~400 lines of
straightforward forward data-flow analysis.
Passing `--nodebug` will disable debug mode, even if only a single fixture is filtered. This can help when running tests one by one and checking if their output is correct - debug mode causes tests to fail because there is extra/different error output.
Uses a simple algorithm for detecting writes of refs during render, since the ref _read_ during render is now handled as part of ValidateNoImpureValuesInRender.
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.

2 participants