useRef: Warn about reading or writing mutable values during render (#18545)

Reading or writing a ref value during render is only safe if you are implementing the lazy initialization pattern. Other types of reading are unsafe as the ref is a mutable source. Other types of writing are unsafe as they are effectively side effects. This change also refactors useTransition to no longer use a ref hook, but instead manage its own (stable) hook state.

Author: Brian Vaughn

packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js

@@ -474,31 +474,29 @@ describe('ReactDOMServerHooks', () => {
 describe('useRef',()=>{
 itRenders('basic render',asyncrender=>{
 functionCounter(props){
-constcount=useRef(0);
-return<span>Count: {count.current}</span>;
+constref=useRef();
+return<spanref={ref}>Hi</span>;
 }
 
 constdomNode=awaitrender(<Counter/>);
-expect(domNode.textContent).toEqual('Count: 0');
+expect(domNode.textContent).toEqual('Hi');
 });
 
 itRenders(
 'multiple times when updates happen during the render phase',
 asyncrender=>{
 functionCounter(props){
 const[count,setCount]=useState(0);
-constref=useRef(count);
+constref=useRef();
 
 if(count<3){
 constnewCount=count+1;
-
-ref.current=newCount;
 setCount(newCount);
 }
 
 yieldValue(count);
 
-return<span>Count: {ref.current}</span>;
+return<spanref={ref}>Count: {count}</span>;
 }
 
 constdomNode=awaitrender(<Counter/>);
@@ -513,7 +511,7 @@ describe('ReactDOMServerHooks', () => {
 letfirstRef=null;
 functionCounter(props){
 const[count,setCount]=useState(0);
-constref=useRef(count);
+constref=useRef();
 if(firstRef===null){
 firstRef=ref;
 }elseif(firstRef!==ref){
@@ -528,12 +526,12 @@ describe('ReactDOMServerHooks', () => {
 
 yieldValue(count);
 
-return<span>Count: {ref.current}</span>;
+return<spanref={ref}>Count: {count}</span>;
 }
 
 constdomNode=awaitrender(<Counter/>);
 expect(clearYields()).toEqual([0,1,2,3]);
-expect(domNode.textContent).toEqual('Count: 0');
+expect(domNode.textContent).toEqual('Count: 3');
 },
 );
 });

packages/react-reconciler/src/ReactFiberHooks.new.js

@@ -27,6 +27,7 @@ import {
 enableNewReconciler,
 decoupleUpdatePriorityFromScheduler,
 enableDoubleInvokingEffects,
+enableUseRefAccessWarning,
 }from'shared/ReactFeatureFlags';
 
 import{
@@ -1197,14 +1198,92 @@ function pushEffect(tag, create, destroy, deps) {
  return effect;
 }
 
+letstackContainsErrorMessage: boolean|null=null;
+
+functiongetCallerStackFrame(): string{
+conststackFrames=newError('Error message').stack.split('\n');
+
+// Some browsers (e.g. Chrome) include the error message in the stack
+// but others (e.g. Firefox) do not.
+if(stackContainsErrorMessage===null){
+ stackContainsErrorMessage =stackFrames[0].includes('Error message');
+}
+
+returnstackContainsErrorMessage
+ ? stackFrames.slice(3,4).join('\n')
+ : stackFrames.slice(2,3).join('\n');
+}
+
 function mountRef<T>(initialValue: T): {|current: T|}{
 consthook=mountWorkInProgressHook();
-constref={current: initialValue};
-if(__DEV__){
-Object.seal(ref);
+if(enableUseRefAccessWarning){
+if(__DEV__){
+// Support lazy initialization pattern shown in docs.
+// We need to store the caller stack frame so that we don't warn on subsequent renders.
+lethasBeenInitialized=initialValue!=null;
+letlazyInitGetterStack=null;
+letdidCheckForLazyInit=false;
+
+// Only warn once per component+hook.
+letdidWarnAboutRead=false;
+letdidWarnAboutWrite=false;
+
+letcurrent=initialValue;
+constref={
+getcurrent(){
+if(!hasBeenInitialized){
+didCheckForLazyInit=true;
+lazyInitGetterStack=getCallerStackFrame();
+}elseif(currentlyRenderingFiber!==null&&!didWarnAboutRead){
+if(
+lazyInitGetterStack===null||
+lazyInitGetterStack!==getCallerStackFrame()
+){
+didWarnAboutRead=true;
+console.warn(
+'%s: Unsafe read of a mutable value during render.\n\n'+
+'Reading from a ref during render is only safe if:\n'+
+'1. The ref value has not been updated, or\n'+
+'2. The ref holds a lazily-initialized value that is only set once.\n',
+getComponentName(currentlyRenderingFiber.type)||'Unknown',
+);
+}
+}
+returncurrent;
+},
+setcurrent(value){
+if(currentlyRenderingFiber!==null&&!didWarnAboutWrite){
+if(
+hasBeenInitialized||
+(!hasBeenInitialized&&!didCheckForLazyInit)
+){
+didWarnAboutWrite=true;
+console.warn(
+'%s: Unsafe write of a mutable value during render.\n\n'+
+'Writing to a ref during render is only safe if the ref holds '+
+'a lazily-initialized value that is only set once.\n',
+getComponentName(currentlyRenderingFiber.type)||'Unknown',
+);
+}
+}
+
+hasBeenInitialized=true;
+current=value;
+},
+};
+Object.seal(ref);
+hook.memoizedState=ref;
+returnref;
+}else{
+constref={current: initialValue};
+hook.memoizedState=ref;
+returnref;
+}
+}else{
+constref={current: initialValue};
+hook.memoizedState=ref;
+returnref;
 }
- hook.memoizedState = ref;
- return ref;
 }
 
 functionupdateRef<T>(initialValue: T): {|current: T|}{
@@ -1591,24 +1670,24 @@ function startTransition(setPending, callback) {
 
 functionmountTransition(): [(()=>void)=>void,boolean]{
 const[isPending,setPending]=mountState(false);
-// The `start` method can be stored on a ref, since `setPending`
-// never changes.
+// The `start` method never changes.
 conststart=startTransition.bind(null,setPending);
-mountRef(start);
+consthook=mountWorkInProgressHook();
+hook.memoizedState=start;
 return[start,isPending];
 }
 
 function updateTransition(): [(() =>void)=>void,boolean]{
 const[isPending]=updateState(false);
-conststartRef=updateRef();
-conststart: (()=>void)=>void=(startRef.current: any);
+consthook=updateWorkInProgressHook();
+conststart=hook.memoizedState;
 return[start,isPending];
 }
 
 function rerenderTransition(): [(() =>void)=>void,boolean]{
 const[isPending]=rerenderState(false);
-conststartRef=updateRef();
-conststart: (()=>void)=>void=(startRef.current: any);
+consthook=updateWorkInProgressHook();
+conststart=hook.memoizedState;
 return[start,isPending];
 }
 

packages/react-reconciler/src/ReactFiberHooks.old.js

@@ -26,6 +26,7 @@ import {
 enableSchedulingProfiler,
 enableNewReconciler,
 decoupleUpdatePriorityFromScheduler,
+enableUseRefAccessWarning,
 }from'shared/ReactFeatureFlags';
 
 import{NoMode,BlockingMode,DebugTracingMode}from'./ReactTypeOfMode';
@@ -1175,14 +1176,92 @@ function pushEffect(tag, create, destroy, deps) {
  return effect;
 }
 
+letstackContainsErrorMessage: boolean|null=null;
+
+functiongetCallerStackFrame(): string{
+conststackFrames=newError('Error message').stack.split('\n');
+
+// Some browsers (e.g. Chrome) include the error message in the stack
+// but others (e.g. Firefox) do not.
+if(stackContainsErrorMessage===null){
+ stackContainsErrorMessage =stackFrames[0].includes('Error message');
+}
+
+returnstackContainsErrorMessage
+ ? stackFrames.slice(3,4).join('\n')
+ : stackFrames.slice(2,3).join('\n');
+}
+
 function mountRef<T>(initialValue: T): {|current: T|}{
 consthook=mountWorkInProgressHook();
-constref={current: initialValue};
-if(__DEV__){
-Object.seal(ref);
+if(enableUseRefAccessWarning){
+if(__DEV__){
+// Support lazy initialization pattern shown in docs.
+// We need to store the caller stack frame so that we don't warn on subsequent renders.
+lethasBeenInitialized=initialValue!=null;
+letlazyInitGetterStack=null;
+letdidCheckForLazyInit=false;
+
+// Only warn once per component+hook.
+letdidWarnAboutRead=false;
+letdidWarnAboutWrite=false;
+
+letcurrent=initialValue;
+constref={
+getcurrent(){
+if(!hasBeenInitialized){
+didCheckForLazyInit=true;
+lazyInitGetterStack=getCallerStackFrame();
+}elseif(currentlyRenderingFiber!==null&&!didWarnAboutRead){
+if(
+lazyInitGetterStack===null||
+lazyInitGetterStack!==getCallerStackFrame()
+){
+didWarnAboutRead=true;
+console.warn(
+'%s: Unsafe read of a mutable value during render.\n\n'+
+'Reading from a ref during render is only safe if:\n'+
+'1. The ref value has not been updated, or\n'+
+'2. The ref holds a lazily-initialized value that is only set once.\n',
+getComponentName(currentlyRenderingFiber.type)||'Unknown',
+);
+}
+}
+returncurrent;
+},
+setcurrent(value){
+if(currentlyRenderingFiber!==null&&!didWarnAboutWrite){
+if(
+hasBeenInitialized||
+(!hasBeenInitialized&&!didCheckForLazyInit)
+){
+didWarnAboutWrite=true;
+console.warn(
+'%s: Unsafe write of a mutable value during render.\n\n'+
+'Writing to a ref during render is only safe if the ref holds '+
+'a lazily-initialized value that is only set once.\n',
+getComponentName(currentlyRenderingFiber.type)||'Unknown',
+);
+}
+}
+
+hasBeenInitialized=true;
+current=value;
+},
+};
+Object.seal(ref);
+hook.memoizedState=ref;
+returnref;
+}else{
+constref={current: initialValue};
+hook.memoizedState=ref;
+returnref;
+}
+}else{
+constref={current: initialValue};
+hook.memoizedState=ref;
+returnref;
 }
- hook.memoizedState = ref;
- return ref;
 }
 
 functionupdateRef<T>(initialValue: T): {|current: T|}{
@@ -1534,24 +1613,24 @@ function startTransition(setPending, callback) {
 
 functionmountTransition(): [(()=>void)=>void,boolean]{
 const[isPending,setPending]=mountState(false);
-// The `start` method can be stored on a ref, since `setPending`
-// never changes.
+// The `start` method never changes.
 conststart=startTransition.bind(null,setPending);
-mountRef(start);
+consthook=mountWorkInProgressHook();
+hook.memoizedState=start;
 return[start,isPending];
 }
 
 function updateTransition(): [(() =>void)=>void,boolean]{
 const[isPending]=updateState(false);
-conststartRef=updateRef();
-conststart: (()=>void)=>void=(startRef.current: any);
+consthook=updateWorkInProgressHook();
+conststart=hook.memoizedState;
 return[start,isPending];
 }
 
 function rerenderTransition(): [(() =>void)=>void,boolean]{
 const[isPending]=rerenderState(false);
-conststartRef=updateRef();
-conststart: (()=>void)=>void=(startRef.current: any);
+consthook=updateWorkInProgressHook();
+conststart=hook.memoizedState;
 return[start,isPending];
 }