Repository: facebook/react
Pull request: [compiler] Don't show hint about ref-like naming if we infer another type


Some components accept a union of a ref callback function or ref object. In this case we may infer the type as a function due to the presence of invoking the ref callback function. In that case, we currently report a "Hint: name `fooRef` as "ref" or with a "-Ref" suffix..." even though the variable is already named appropriately — the problem is that we inferred a non-ref type. So here we check the type and don't report this hint if we inferred another type.


Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
patch: @@ -1816,8 +1816,16 @@ function computeSignatureForInstruction(
     }
     case 'PropertyStore':
     case 'ComputedStore': {
+      /**
+       * Add a hint about naming as "ref"/"-Ref", but only if we weren't able to infer any
+       * type for the object. In some cases the variable may be named like a ref, but is
+       * also used as a ref callback such that we infer the type as a function rather than
+       * a ref.
+       */
       const mutationReason: MutationReason | null =
-        value.kind === 'PropertyStore' && value.property === 'current'
+        value.kind === 'PropertyStore' &&
+        value.property === 'current' &&
+        value.object.identifier.type.kind === 'Type'
           ? {kind: 'AssignCurrentProperty'}
           : null;
       effects.push({

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.expect.md
patch: @@ -0,0 +1,48 @@
+
+## Input
+
+```javascript
+// @validateRefAccessDuringRender
+
+function useHook(parentRef) {
+  // Some components accept a union of "callback" refs and ref objects, which
+  // we can't currently represent
+  const elementRef = useRef(null);
+  const handler = instance => {
+    elementRef.current = instance;
+    if (parentRef != null) {
+      if (typeof parentRef === 'function') {
+        // This call infers the type of `parentRef` as a function...
+        parentRef(instance);
+      } else {
+        // So this assignment fails since we don't know its a ref
+        parentRef.current = instance;
+      }
+    }
+  };
+  return handler;
+}
+
+```
+
+
+## Error
+
+```
+Found 1 error:
+
+Error: This value cannot be modified
+
+Modifying component props or hook arguments is not allowed. Consider using a local variable instead.
+
+error.todo-allow-assigning-to-inferred-ref-prop-in-callback.ts:15:8
+  13 |       } else {
+  14 |         // So this assignment fails since we don't know its a ref
+> 15 |         parentRef.current = instance;
+     |         ^^^^^^^^^ `parentRef` cannot be modified
+  16 |       }
+  17 |     }
+  18 |   };
+```
+          
+      
\ No newline at end of file

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.js
patch: @@ -0,0 +1,20 @@
+// @validateRefAccessDuringRender
+
+function useHook(parentRef) {
+  // Some components accept a union of "callback" refs and ref objects, which
+  // we can't currently represent
+  const elementRef = useRef(null);
+  const handler = instance => {
+    elementRef.current = instance;
+    if (parentRef != null) {
+      if (typeof parentRef === 'function') {
+        // This call infers the type of `parentRef` as a function...
+        parentRef(instance);
+      } else {
+        // So this assignment fails since we don't know its a ref
+        parentRef.current = instance;
+      }
+    }
+  };
+  return handler;
+}