Update

[ghstack-poisoned]

Author: poteto

ReactVersions.js

@@ -52,7 +52,7 @@ const stablePackages = {
 // These packages do not exist in the @canary or @latest channel, only
 // @experimental. We don't use semver, just the commit sha, so this is just a
 // list of package names instead of a map.
-const experimentalPackages = [];
+const experimentalPackages = ['react-markup'];
 
 module.exports = {
   ReactVersion,

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

@@ -223,7 +223,7 @@ const EnvironmentConfigSchema = z.object({
   validateHooksUsage: z.boolean().default(true),
 
   // Validate that ref values (`ref.current`) are not accessed during render.
-  validateRefAccessDuringRender: z.boolean().default(false),
+  validateRefAccessDuringRender: z.boolean().default(true),
 
   /*
    * Validates that setState is not unconditionally called during render, as it can lead to

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

@@ -1591,6 +1591,10 @@ export function isUseStateType(id: Identifier): boolean {
   return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInUseState';
 }
 
+export function isRefOrRefValue(id: Identifier): boolean {
+  return isUseRefType(id) || isRefValueType(id);
+}
+
 export function isSetStateType(id: Identifier): boolean {
   return id.type.kind === 'Function' && id.type.shapeId === 'BuiltInSetState';
 }

compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts

@@ -14,8 +14,7 @@ import {
   LoweredFunction,
   Place,
   ReactiveScopeDependency,
-  isRefValueType,
-  isUseRefType,
+  isRefOrRefValue,
   makeInstructionId,
 } from '../HIR';
 import {deadCodeElimination} from '../Optimization';
@@ -139,7 +138,7 @@ function infer(
       name = dep.identifier.name;
     }
 
-    if (isUseRefType(dep.identifier) || isRefValueType(dep.identifier)) {
+    if (isRefOrRefValue(dep.identifier)) {
       /*
        * TODO: this is a hack to ensure we treat functions which reference refs
        * as having a capture and therefore being considered mutable. this ensures

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts

@@ -11,6 +11,7 @@ import {
   Identifier,
   InstructionId,
   InstructionKind,
+  isRefOrRefValue,
   makeInstructionId,
   Place,
 } from '../HIR/HIR';
@@ -66,7 +67,9 @@ import {assertExhaustive} from '../Utils/utils';
  */
 
 function infer(place: Place, instrId: InstructionId): void {
-  place.identifier.mutableRange.end = makeInstructionId(instrId + 1);
+  if (!isRefOrRefValue(place.identifier)) {
+    place.identifier.mutableRange.end = makeInstructionId(instrId + 1);
+  }
 }
 
 function inferPlace(
@@ -171,7 +174,10 @@ export function inferMutableLifetimes(
         const declaration = contextVariableDeclarationInstructions.get(
           instr.value.lvalue.place.identifier,
         );
-        if (declaration != null) {
+        if (
+          declaration != null &&
+          !isRefOrRefValue(instr.value.lvalue.place.identifier)
+        ) {
           const range = instr.value.lvalue.place.identifier.mutableRange;
           if (range.start === 0) {
             range.start = declaration;

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForAlias.ts

@@ -5,7 +5,12 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {HIRFunction, Identifier, InstructionId} from '../HIR/HIR';
+import {
+  HIRFunction,
+  Identifier,
+  InstructionId,
+  isRefOrRefValue,
+} from '../HIR/HIR';
 import DisjointSet from '../Utils/DisjointSet';
 
 export function inferMutableRangesForAlias(
@@ -19,7 +24,8 @@ export function inferMutableRangesForAlias(
      * mutated.
      */
     const mutatingIdentifiers = [...aliasSet].filter(
-      id => id.mutableRange.end - id.mutableRange.start > 1,
+      id =>
+        id.mutableRange.end - id.mutableRange.start > 1 && !isRefOrRefValue(id),
     );
 
     if (mutatingIdentifiers.length > 0) {
@@ -36,7 +42,10 @@ export function inferMutableRangesForAlias(
        * last mutation.
        */
       for (const alias of aliasSet) {
-        if (alias.mutableRange.end < lastMutatingInstructionId) {
+        if (
+          alias.mutableRange.end < lastMutatingInstructionId &&
+          !isRefOrRefValue(alias)
+        ) {
           alias.mutableRange.end = lastMutatingInstructionId as InstructionId;
         }
       }

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts

@@ -30,8 +30,7 @@ import {
   isArrayType,
   isMutableEffect,
   isObjectType,
-  isRefValueType,
-  isUseRefType,
+  isRefOrRefValue,
 } from '../HIR/HIR';
 import {FunctionSignature} from '../HIR/ObjectShape';
 import {
@@ -523,10 +522,7 @@ class InferenceState {
         break;
       }
       case Effect.Mutate: {
-        if (
-          isRefValueType(place.identifier) ||
-          isUseRefType(place.identifier)
-        ) {
+        if (isRefOrRefValue(place.identifier)) {
           // no-op: refs are validate via ValidateNoRefAccessInRender
         } else if (valueKind.kind === ValueKind.Context) {
           functionEffect = {
@@ -567,10 +563,7 @@ class InferenceState {
         break;
       }
       case Effect.Store: {
-        if (
-          isRefValueType(place.identifier) ||
-          isUseRefType(place.identifier)
-        ) {
+        if (isRefOrRefValue(place.identifier)) {
           // no-op: refs are validate via ValidateNoRefAccessInRender
         } else if (valueKind.kind === ValueKind.Context) {
           functionEffect = {

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

@@ -11,10 +11,10 @@ import {
   IdentifierId,
   Place,
   SourceLocation,
+  isRefOrRefValue,
   isRefValueType,
   isUseRefType,
 } from '../HIR';
-import {printPlace} from '../HIR/PrintHIR';
 import {
   eachInstructionValueOperand,
   eachTerminalOperand,
@@ -52,41 +52,50 @@ function validateNoRefAccessInRenderImpl(
   refAccessingFunctions: Set<IdentifierId>,
 ): Result<void, CompilerError> {
   const errors = new CompilerError();
+  const lookupLocations: Map<IdentifierId, SourceLocation> = new Map();
   for (const [, block] of fn.body.blocks) {
     for (const instr of block.instructions) {
       switch (instr.value.kind) {
         case 'JsxExpression':
         case 'JsxFragment': {
           for (const operand of eachInstructionValueOperand(instr.value)) {
-            if (isRefValueType(operand.identifier)) {
-              errors.push({
-                severity: ErrorSeverity.InvalidReact,
-                reason:
-                  'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-                loc: operand.loc,
-                description: `Cannot access ref value at ${printPlace(
-                  operand,
-                )}`,
-                suggestions: null,
-              });
-            }
+            validateNoDirectRefValueAccess(errors, operand, lookupLocations);
           }
           break;
         }
         case 'PropertyLoad': {
+          if (
+            isRefValueType(instr.lvalue.identifier) &&
+            instr.value.property === 'current'
+          ) {
+            lookupLocations.set(instr.lvalue.identifier.id, instr.loc);
+          }
           break;
         }
         case 'LoadLocal': {
           if (refAccessingFunctions.has(instr.value.place.identifier.id)) {
             refAccessingFunctions.add(instr.lvalue.identifier.id);
           }
+          if (isRefValueType(instr.lvalue.identifier)) {
+            const loc = lookupLocations.get(instr.value.place.identifier.id);
+            if (loc !== undefined) {
+              lookupLocations.set(instr.lvalue.identifier.id, loc);
+            }
+          }
           break;
         }
         case 'StoreLocal': {
           if (refAccessingFunctions.has(instr.value.value.identifier.id)) {
             refAccessingFunctions.add(instr.value.lvalue.place.identifier.id);
             refAccessingFunctions.add(instr.lvalue.identifier.id);
           }
+          if (isRefValueType(instr.value.lvalue.place.identifier)) {
+            const loc = lookupLocations.get(instr.value.value.identifier.id);
+            if (loc !== undefined) {
+              lookupLocations.set(instr.value.lvalue.place.identifier.id, loc);
+              lookupLocations.set(instr.lvalue.identifier.id, loc);
+            }
+          }
           break;
         }
         case 'ObjectMethod':
@@ -139,7 +148,11 @@ function validateNoRefAccessInRenderImpl(
                 reason:
                   'This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)',
                 loc: callee.loc,
-                description: `Function ${printPlace(callee)} accesses a ref`,
+                description:
+                  callee.identifier.name !== null &&
+                  callee.identifier.name.kind === 'named'
+                    ? `Function \`${callee.identifier.name.value}\` accesses a ref`
+                    : null,
                 suggestions: null,
               });
             }
@@ -148,7 +161,7 @@ function validateNoRefAccessInRenderImpl(
                 errors,
                 refAccessingFunctions,
                 operand,
-                operand.loc,
+                lookupLocations.get(operand.identifier.id) ?? operand.loc,
               );
             }
           }
@@ -161,7 +174,7 @@ function validateNoRefAccessInRenderImpl(
               errors,
               refAccessingFunctions,
               operand,
-              operand.loc,
+              lookupLocations.get(operand.identifier.id) ?? operand.loc,
             );
           }
           break;
@@ -174,26 +187,49 @@ function validateNoRefAccessInRenderImpl(
             errors,
             refAccessingFunctions,
             instr.value.object,
-            instr.loc,
+            lookupLocations.get(instr.value.object.identifier.id) ?? instr.loc,
           );
           for (const operand of eachInstructionValueOperand(instr.value)) {
             if (operand === instr.value.object) {
               continue;
             }
-            validateNoRefValueAccess(errors, refAccessingFunctions, operand);
+            validateNoRefValueAccess(
+              errors,
+              refAccessingFunctions,
+              lookupLocations,
+              operand,
+            );
           }
           break;
         }
+        case 'StartMemoize':
+        case 'FinishMemoize':
+          break;
         default: {
           for (const operand of eachInstructionValueOperand(instr.value)) {
-            validateNoRefValueAccess(errors, refAccessingFunctions, operand);
+            validateNoRefValueAccess(
+              errors,
+              refAccessingFunctions,
+              lookupLocations,
+              operand,
+            );
           }
           break;
         }
       }
     }
     for (const operand of eachTerminalOperand(block.terminal)) {
-      validateNoRefValueAccess(errors, refAccessingFunctions, operand);
+      if (block.terminal.kind !== 'return') {
+        validateNoRefValueAccess(
+          errors,
+          refAccessingFunctions,
+          lookupLocations,
+          operand,
+        );
+      } else {
+        // Allow functions containing refs to be returned, but not direct ref values
+        validateNoDirectRefValueAccess(errors, operand, lookupLocations);
+      }
     }
   }
 
@@ -207,6 +243,7 @@ function validateNoRefAccessInRenderImpl(
 function validateNoRefValueAccess(
   errors: CompilerError,
   refAccessingFunctions: Set<IdentifierId>,
+  lookupLocations: Map<IdentifierId, SourceLocation>,
   operand: Place,
 ): void {
   if (
@@ -217,8 +254,12 @@ function validateNoRefValueAccess(
       severity: ErrorSeverity.InvalidReact,
       reason:
         'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: operand.loc,
-      description: `Cannot access ref value at ${printPlace(operand)}`,
+      loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
+      description:
+        operand.identifier.name !== null &&
+        operand.identifier.name.kind === 'named'
+          ? `Cannot access ref value \`${operand.identifier.name.value}\``
+          : null,
       suggestions: null,
     });
   }
@@ -231,8 +272,7 @@ function validateNoRefAccess(
   loc: SourceLocation,
 ): void {
   if (
-    isRefValueType(operand.identifier) ||
-    isUseRefType(operand.identifier) ||
+    isRefOrRefValue(operand.identifier) ||
     refAccessingFunctions.has(operand.identifier.id)
   ) {
     errors.push({
@@ -249,3 +289,24 @@ function validateNoRefAccess(
     });
   }
 }
+
+function validateNoDirectRefValueAccess(
+  errors: CompilerError,
+  operand: Place,
+  lookupLocations: Map<IdentifierId, SourceLocation>,
+): void {
+  if (isRefValueType(operand.identifier)) {
+    errors.push({
+      severity: ErrorSeverity.InvalidReact,
+      reason:
+        'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
+      loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
+      description:
+        operand.identifier.name !== null &&
+        operand.identifier.name.kind === 'named'
+          ? `Cannot access ref value \`${operand.identifier.name.value}\``
+          : null,
+      suggestions: null,
+    });
+  }
+}