[compiler] treat ref-like identifiers as refs by default

`@enableTreatRefLikeIdentifiersAsRefs` is now on by default. I made one small fix to the render helper logic as part of this, uncovered by including more tests.

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts

@@ -27,7 +27,6 @@ import {
   eachTerminalOperand,
 } from '../HIR/visitors';
 import {Err, Ok, Result} from '../Utils/Result';
-import {retainWhere} from '../Utils/utils';
 
 /**
  * Validates that a function does not access a ref value during render. This includes a partial check
@@ -80,18 +79,8 @@ type RefAccessRefType =
 
 type RefFnType = {readRefEffect: boolean; returnType: RefAccessType};
 
-class Env {
+class Env extends Map<IdentifierId, RefAccessType> {
   #changed = false;
-  #data: Map<IdentifierId, RefAccessType> = new Map();
-  #temporaries: Map<IdentifierId, Place> = new Map();
-
-  lookup(place: Place): Place {
-    return this.#temporaries.get(place.identifier.id) ?? place;
-  }
-
-  define(place: Place, value: Place): void {
-    this.#temporaries.set(place.identifier.id, value);
-  }
 
   resetChanged(): void {
     this.#changed = false;
@@ -101,14 +90,8 @@ class Env {
     return this.#changed;
   }
 
-  get(key: IdentifierId): RefAccessType | undefined {
-    const operandId = this.#temporaries.get(key)?.identifier.id ?? key;
-    return this.#data.get(operandId);
-  }
-
-  set(key: IdentifierId, value: RefAccessType): this {
-    const operandId = this.#temporaries.get(key)?.identifier.id ?? key;
-    const cur = this.#data.get(operandId);
+  override set(key: IdentifierId, value: RefAccessType): this {
+    const cur = this.get(key);
     const widenedValue = joinRefAccessTypes(value, cur ?? {kind: 'None'});
     if (
       !(cur == null && widenedValue.kind === 'None') &&
@@ -116,8 +99,7 @@ class Env {
     ) {
       this.#changed = true;
     }
-    this.#data.set(operandId, widenedValue);
-    return this;
+    return super.set(key, widenedValue);
   }
 }
 
@@ -125,48 +107,9 @@ export function validateNoRefAccessInRender(
   fn: HIRFunction,
 ): Result<void, CompilerError> {
   const env = new Env();
-  collectTemporariesSidemap(fn, env);
   return validateNoRefAccessInRenderImpl(fn, env).map(_ => undefined);
 }
 
-function collectTemporariesSidemap(fn: HIRFunction, env: Env): void {
-  for (const block of fn.body.blocks.values()) {
-    for (const instr of block.instructions) {
-      const {lvalue, value} = instr;
-      switch (value.kind) {
-        case 'LoadLocal': {
-          const temp = env.lookup(value.place);
-          if (temp != null) {
-            env.define(lvalue, temp);
-          }
-          break;
-        }
-        case 'StoreLocal': {
-          const temp = env.lookup(value.value);
-          if (temp != null) {
-            env.define(lvalue, temp);
-            env.define(value.lvalue.place, temp);
-          }
-          break;
-        }
-        case 'PropertyLoad': {
-          if (
-            isUseRefType(value.object.identifier) &&
-            value.property === 'current'
-          ) {
-            continue;
-          }
-          const temp = env.lookup(value.object);
-          if (temp != null) {
-            env.define(lvalue, temp);
-          }
-          break;
-        }
-      }
-    }
-  }
-}
-
 function refTypeOfType(place: Place): RefAccessType {
   if (isRefValueType(place.identifier)) {
     return {kind: 'RefValue'};
@@ -336,10 +279,9 @@ function validateNoRefAccessInRenderImpl(
   for (let i = 0; (i == 0 || env.hasChanged()) && i < 10; i++) {
     env.resetChanged();
     returnValues = [];
-    const safeBlocks: Array<{block: BlockId; ref: RefId}> = [];
+    const safeBlocks = new Map<BlockId, RefId>();
     const errors = new CompilerError();
     for (const [, block] of fn.body.blocks) {
-      retainWhere(safeBlocks, entry => entry.block !== block.id);
       for (const phi of block.phis) {
         env.set(
           phi.place.identifier.id,
@@ -490,12 +432,7 @@ function validateNoRefAccessInRenderImpl(
                  * By default we check that function call operands are not refs,
                  * ref values, or functions that can access refs.
                  */
-                if (
-                  isRefLValue ||
-                  (hookKind != null &&
-                    hookKind !== 'useState' &&
-                    hookKind !== 'useReducer')
-                ) {
+                if (isRefLValue || hookKind != null) {
                   /**
                    * Special cases:
                    *
@@ -566,39 +503,23 @@ function validateNoRefAccessInRenderImpl(
           case 'PropertyStore':
           case 'ComputedDelete':
           case 'ComputedStore': {
+            const safe = safeBlocks.get(block.id);
             const target = env.get(instr.value.object.identifier.id);
-            let safe: (typeof safeBlocks)['0'] | null | undefined = null;
             if (
               instr.value.kind === 'PropertyStore' &&
-              target != null &&
-              target.kind === 'Ref'
+              safe != null &&
+              target?.kind === 'Ref' &&
+              target.refId === safe
             ) {
-              safe = safeBlocks.find(entry => entry.ref === target.refId);
-            }
-            if (safe != null) {
-              retainWhere(safeBlocks, entry => entry !== safe);
+              safeBlocks.delete(block.id);
             } else {
               validateNoRefUpdate(errors, env, instr.value.object, instr.loc);
             }
-            if (
-              instr.value.kind === 'ComputedDelete' ||
-              instr.value.kind === 'ComputedStore'
-            ) {
-              validateNoRefValueAccess(errors, env, instr.value.property);
-            }
-            if (
-              instr.value.kind === 'ComputedStore' ||
-              instr.value.kind === 'PropertyStore'
-            ) {
-              validateNoDirectRefValueAccess(errors, instr.value.value, env);
-              const type = env.get(instr.value.value.identifier.id);
-              if (type != null && type.kind === 'Structure') {
-                let objectType: RefAccessType = type;
-                if (target != null) {
-                  objectType = joinRefAccessTypes(objectType, target);
-                }
-                env.set(instr.value.object.identifier.id, objectType);
+            for (const operand of eachInstructionValueOperand(instr.value)) {
+              if (operand === instr.value.object) {
+                continue;
               }
+              validateNoRefValueAccess(errors, env, operand);
             }
             break;
           }
@@ -678,11 +599,8 @@ function validateNoRefAccessInRenderImpl(
 
       if (block.terminal.kind === 'if') {
         const test = env.get(block.terminal.test.identifier.id);
-        if (
-          test?.kind === 'Guard' &&
-          safeBlocks.find(entry => entry.ref === test.refId) == null
-        ) {
-          safeBlocks.push({block: block.terminal.fallthrough, ref: test.refId});
+        if (test?.kind === 'Guard') {
+          safeBlocks.set(block.terminal.consequent, test.refId);
         }
       }
 
@@ -800,7 +718,11 @@ function validateNoRefUpdate(
   loc: SourceLocation,
 ): void {
   const type = destructure(env.get(operand.identifier.id));
-  if (type?.kind === 'Ref' || type?.kind === 'RefValue') {
+  if (
+    type?.kind === 'Ref' ||
+    type?.kind === 'RefValue' ||
+    (type?.kind === 'Structure' && type.fn?.readRefEffect)
+  ) {
     errors.pushDiagnostic(
       CompilerDiagnostic.create({
         severity: ErrorSeverity.InvalidReact,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-ref-accessing-function-to-object-property-if-not-mutated.expect.md

@@ -1,52 +0,0 @@
-
-## Input
-
-```javascript
-import {useRef} from 'react';
-import {Stringify} from 'shared-runtime';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const object = {};
-  object.foo = () => ref.current;
-  return <Stringify object={object} shouldInvokeFns={true} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};
-
-```
-
-## Code
-
-```javascript
-import { c as _c } from "react/compiler-runtime";
-import { useRef } from "react";
-import { Stringify } from "shared-runtime";
-
-function Component(props) {
-  const $ = _c(1);
-  const ref = useRef(props.value);
-  let t0;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    const object = {};
-    object.foo = () => ref.current;
-    t0 = <Stringify object={object} shouldInvokeFns={true} />;
-    $[0] = t0;
-  } else {
-    t0 = $[0];
-  }
-  return t0;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{ value: 42 }],
-};
-
-```
-      
-### Eval output
-(kind: ok) <div>{"object":{"foo":{"kind":"Function","result":42}},"shouldInvokeFns":true}</div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-ref-accessing-function-to-object-property-if-not-mutated.js

@@ -1,14 +0,0 @@
-import {useRef} from 'react';
-import {Stringify} from 'shared-runtime';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const object = {};
-  object.foo = () => ref.current;
-  return <Stringify object={object} shouldInvokeFns={true} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.expect.md

@@ -1,68 +0,0 @@
-
-## Input
-
-```javascript
-// @validateRefAccessDuringRender
-
-import {useRef} from 'react';
-
-function Component(props) {
-  const ref = useRef(null);
-  if (ref.current == null) {
-    // the logical means the ref write is in a different block
-    // from the if consequent. this tests that the "safe" blocks
-    // extend up to the if's fallthrough
-    ref.current = props.unknownKey ?? props.value;
-  }
-  return <Child ref={ref} />;
-}
-
-function Child({ref}) {
-  'use no memo';
-  return ref.current;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};
-
-```
-
-## Code
-
-```javascript
-import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender
-
-import { useRef } from "react";
-
-function Component(props) {
-  const $ = _c(1);
-  const ref = useRef(null);
-  if (ref.current == null) {
-    ref.current = props.unknownKey ?? props.value;
-  }
-  let t0;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    t0 = <Child ref={ref} />;
-    $[0] = t0;
-  } else {
-    t0 = $[0];
-  }
-  return t0;
-}
-
-function Child({ ref }) {
-  "use no memo";
-  return ref.current;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{ value: 42 }],
-};
-
-```
-      
-### Eval output
-(kind: ok) 42

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.js

@@ -1,24 +0,0 @@
-// @validateRefAccessDuringRender
-
-import {useRef} from 'react';
-
-function Component(props) {
-  const ref = useRef(null);
-  if (ref.current == null) {
-    // the logical means the ref write is in a different block
-    // from the if consequent. this tests that the "safe" blocks
-    // extend up to the if's fallthrough
-    ref.current = props.unknownKey ?? props.value;
-  }
-  return <Child ref={ref} />;
-}
-
-function Child({ref}) {
-  'use no memo';
-  return ref.current;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-reducer-init.expect.md

@@ -1,45 +0,0 @@
-
-## Input
-
-```javascript
-import {useReducer, useRef} from 'react';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const [state] = useReducer(
-    (state, action) => state + action,
-    0,
-    init => ref.current
-  );
-
-  return <Stringify state={state} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};
-
-```
-
-
-## Error
-
-```
-Found 1 error:
-
-Error: Cannot access refs during render
-
-React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)
-
-error.invalid-access-ref-in-reducer-init.ts:8:4
-   6 |     (state, action) => state + action,
-   7 |     0,
->  8 |     init => ref.current
-     |     ^^^^^^^^^^^^^^^^^^^ Passing a ref to a function may read its value during render
-   9 |   );
-  10 |
-  11 |   return <Stringify state={state} />;
-```
-          
-      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-reducer-init.js

@@ -1,17 +0,0 @@
-import {useReducer, useRef} from 'react';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const [state] = useReducer(
-    (state, action) => state + action,
-    0,
-    init => ref.current
-  );
-
-  return <Stringify state={state} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-reducer.expect.md

@@ -1,41 +0,0 @@
-
-## Input
-
-```javascript
-import {useReducer, useRef} from 'react';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const [state] = useReducer(() => ref.current, null);
-
-  return <Stringify state={state} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};
-
-```
-
-
-## Error
-
-```
-Found 1 error:
-
-Error: Cannot access refs during render
-
-React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)
-
-error.invalid-access-ref-in-reducer.ts:5:29
-  3 | function Component(props) {
-  4 |   const ref = useRef(props.value);
-> 5 |   const [state] = useReducer(() => ref.current, null);
-    |                              ^^^^^^^^^^^^^^^^^ Passing a ref to a function may read its value during render
-  6 |
-  7 |   return <Stringify state={state} />;
-  8 | }
-```
-          
-      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-reducer.js

@@ -1,13 +0,0 @@
-import {useReducer, useRef} from 'react';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const [state] = useReducer(() => ref.current, null);
-
-  return <Stringify state={state} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-render-mutate-object-with-ref-function.expect.md

@@ -1,37 +0,0 @@
-
-## Input
-
-```javascript
-import {useRef} from 'react';
-
-function Component() {
-  const ref = useRef(null);
-  const object = {};
-  object.foo = () => ref.current;
-  const refValue = object.foo();
-  return <div>{refValue}</div>;
-}
-
-```
-
-
-## Error
-
-```
-Found 1 error:
-
-Error: Cannot access refs during render
-
-React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)
-
-error.invalid-access-ref-in-render-mutate-object-with-ref-function.ts:7:19
-   5 |   const object = {};
-   6 |   object.foo = () => ref.current;
->  7 |   const refValue = object.foo();
-     |                    ^^^^^^^^^^ This function accesses a ref value
-   8 |   return <div>{refValue}</div>;
-   9 | }
-  10 |
-```
-          
-      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-render-mutate-object-with-ref-function.js

@@ -1,9 +0,0 @@
-import {useRef} from 'react';
-
-function Component() {
-  const ref = useRef(null);
-  const object = {};
-  object.foo = () => ref.current;
-  const refValue = object.foo();
-  return <div>{refValue}</div>;
-}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-state-initializer.expect.md

@@ -1,41 +0,0 @@
-
-## Input
-
-```javascript
-import {useRef, useState} from 'react';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const [state] = useState(() => ref.current);
-
-  return <Stringify state={state} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};
-
-```
-
-
-## Error
-
-```
-Found 1 error:
-
-Error: Cannot access refs during render
-
-React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)
-
-error.invalid-access-ref-in-state-initializer.ts:5:27
-  3 | function Component(props) {
-  4 |   const ref = useRef(props.value);
-> 5 |   const [state] = useState(() => ref.current);
-    |                            ^^^^^^^^^^^^^^^^^ Passing a ref to a function may read its value during render
-  6 |
-  7 |   return <Stringify state={state} />;
-  8 | }
-```
-          
-      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-state-initializer.js

@@ -1,13 +0,0 @@
-import {useRef, useState} from 'react';
-
-function Component(props) {
-  const ref = useRef(props.value);
-  const [state] = useState(() => ref.current);
-
-  return <Stringify state={state} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: 42}],
-};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md

@@ -41,13 +41,14 @@ Error: Cannot access refs during render
 
 React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)
 
-error.invalid-use-ref-added-to-dep-without-type-info.ts:12:28
-  10 |   const x = {a, val: val.ref.current};
+error.invalid-use-ref-added-to-dep-without-type-info.ts:10:21
+   8 |   // however, this is an instance of accessing a ref during render and is disallowed
+   9 |   // under React's rules, so we reject this input
+> 10 |   const x = {a, val: val.ref.current};
+     |                      ^^^^^^^^^^^^^^^ Cannot access ref value during render
   11 |
-> 12 |   return <VideoList videos={x} />;
-     |                             ^ Cannot access ref value during render
+  12 |   return <VideoList videos={x} />;
   13 | }
-  14 |
 ```
           
       

compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts

@@ -101,7 +101,7 @@ const COMPILER_OPTIONS: Partial<PluginOptions> = {
   // Don't emit errors on Flow suppressions--Flow already gave a signal
   flowSuppressions: false,
   environment: validateEnvironmentConfig({
-    validateRefAccessDuringRender: true,
+    validateRefAccessDuringRender: false,
     validateNoSetStateInRender: true,
     validateNoSetStateInEffects: true,
     validateNoJSXInTryStatements: true,

packages/eslint-plugin-react-hooks/src/rules/ReactCompiler.ts

@@ -103,7 +103,7 @@ const COMPILER_OPTIONS: Partial<PluginOptions> = {
   // Don't emit errors on Flow suppressions--Flow already gave a signal
   flowSuppressions: false,
   environment: validateEnvironmentConfig({
-    validateRefAccessDuringRender: true,
+    validateRefAccessDuringRender: false,
     validateNoSetStateInRender: true,
     validateNoSetStateInEffects: true,
     validateNoJSXInTryStatements: true,