Bug: Derived value does not update in StrictMode

[amanmahajan7]

React version:
16.12.0

Steps To Reproduce

1. Enter a valid number in the textbox that uses strict mode. Notice the value changes in the textbox that does not use strict mode
2. Enter a valid number in the textbox that does not use strict mode. Notice the value does not change in the textbox that uses strict mode

Link to code example:
https://codesandbox.io/s/elated-mendeleev-1m5tm

The current behavior

Derived value does not update in strict mode

The expected behavior

Derived value should update in strict mode

[nstepien]

FWIW it works fine with the production build of react-dom, change the import in codesandbox to test it:

import ReactDOM from "react-dom/cjs/react-dom.production.min";

[bvaughn]

When StrictMode triggers different behavior, it's usually a sign of a side effect or an unsafe pattern that maybe just happens to work in most cases.

What went wrong here?

StrictMode intentionally tests for this by double-invoking render methods. This means that, in DEV mode, React will render a component once- throw away any state updates- and then render again. (Regardless of how many times a component is rendered, the observable behavior should be the same unless there are side effects.)

In this particular case, mutating a ref during render is the side effect. The way this plays out in practice is like so:

1. "loose" input changes e.g. 1 -> 2
2. "strict" input renders for the first time
   1. It changes prevValue ref from 1 -> 2 (side effect) and also updates state setInternalValue("")
   2. It immediately re-renders with the newer state value (internalValue 1 -> 2)
3. React resets component state (including the update setInternalValue("")) in preparation for the second render. (Remember, StrictMode renders twice.)
4. "strict" input renders for the second time
   1. Since the ref was mutated during the first render, it's already the same as the "new" prop so the code you expect to update the internalValue state doesn't get called.

How can you fix it?

The "quick fix" here would be to get rid of the render-phase side effect by moving your ref update to an effect:

if (prevValue.current !== value && internalValue !== "") {
  setInternalValue("");
}

useEffect(() => {
  // It's safe to update your prev-value ref here
  prevValue.current = value;
});

However, this Code Sandbox also looks like one of the derived state anti-patterns described in a blog post a while back. Maybe worth re-considering the higher level approach being used here and possibly going with a fully-controlled component instead?

Why does this matter?

It might not be obvious why this matters. After all- if React didn't double-render the component, it wouldn't break. That's why using the production bundle "fixes" things.

The truth is that side effects like this might cause things to break even outside of strict mode. The upcoming "concurrent" rendering mode is one such case, but it's not the only one. A component might render twice between commits even in "legacy" (synchronous) rendering mode in the event of an error. (In that case, React would bubble the error up to the nearest error boundary, then re-render before committing.)

[amanmahajan7]

Thank you @bvaughn for the quick reply and detailed explanation.

[gmoniava]

Since the ref was mutated during the first render, it's already the same as the "new" prop

@bvaughn when react renders second time intentionally due to strict mode, does it keep the value of ref from previous render, is that what you say? I had a similar situation: https://stackoverflow.com/questions/69393312/state-is-not-updated-in-strictmode, and I thought on second render react discarded the ref value too (hence when I used ref in my question instead of external variable, it seemed to work).

[bvaughn]

Ref isn't changed between regular and strict renders. Shouldn't matter.

Your example code has a side effect (setTimeout) which should not happen in render. Move this to an effect.

[gmoniava]

@bvaughn in my question if you rendered it without strict mode, you would see Paul on screen. If you rendered it with strict mode, you'd see Nick. We thought this was due to using firstRender external variable. firstRender gets modified during first render, and on next strict render, it has different value, hence the second time timeout doesn't fire- and always Nick is printed. Do you agree with this?

If yes, then I found out that by replacing firstRender with a ref, even in strict mode, the output would be now the expected Paul. user ray suggested this could be because unlike firstRender variable, the ref was reinitialized on the second strict render. Hence on the second render the timeout was trigerred. Do you agree with this?

Check the answer and comments.

[bvaughn]

Reading or writing from external values during render is a side effect and is not supported in React components for reasons explained here:
https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

[gmoniava]

@bvaughn That's a general guideline, I see.

But if you could answer more precisely, whether you agree with the explanation (which I summarized in previous comment) user ray came up with, would be nice.

Because like I said we had the impression during second strict render the ref value from previous render was discarded. But in this thread it seemed you claimed ref value was retained on second strict render, and that contradiction confused me.

[bvaughn]

React renders twice in strict mode, without resetting anything:

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

Lines 971 to 1000 in 1e247ff

	if (__DEV__) {
	ReactCurrentOwner.current = workInProgress;
	setIsRendering(true);
	nextChildren = renderWithHooks(
	current,
	workInProgress,
	Component,
	nextProps,
	context,
	renderLanes,
	);
	if (
	debugRenderPhaseSideEffectsForStrictMode &&
	workInProgress.mode & StrictLegacyMode
	) {
	setIsStrictModeForDevtools(true);
	try {
	nextChildren = renderWithHooks(
	current,
	workInProgress,
	Component,
	nextProps,
	context,
	renderLanes,
	);
	} finally {
	setIsStrictModeForDevtools(false);
	}
	}
	setIsRendering(false);

If it's an update, useRef will do exactly the same thing:

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

Lines 1632 to 1635 in 1e247ff

	function updateRef<T>(initialValue: T): {|current: T|} {
	const hook = updateWorkInProgressHook();
	return hook.memoizedState;
	}

If it's a mount, useRef will be initialized twice (re-initialized before strict mode):

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

Lines 1560 to 1630 in 1e247ff

	function mountRef<T>(initialValue: T): {|current: T|} {
	const hook = mountWorkInProgressHook();
	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.
	let hasBeenInitialized = initialValue != null;
	let lazyInitGetterStack = null;
	let didCheckForLazyInit = false;
	// Only warn once per component+hook.
	let didWarnAboutRead = false;
	let didWarnAboutWrite = false;
	let current = initialValue;
	const ref = {
	get current() {
	if (!hasBeenInitialized) {
	didCheckForLazyInit = true;
	lazyInitGetterStack = getCallerStackFrame();
	} else if (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',
	getComponentNameFromFiber(currentlyRenderingFiber) || 'Unknown',
	);
	}
	}
	return current;
	},
	set current(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',
	getComponentNameFromFiber(currentlyRenderingFiber) || 'Unknown',
	);
	}
	}
	hasBeenInitialized = true;
	current = value;
	},
	};
	Object.seal(ref);
	hook.memoizedState = ref;
	return ref;
	} else {
	const ref = {current: initialValue};
	hook.memoizedState = ref;
	return ref;
	}
	} else {
	const ref = {current: initialValue};
	hook.memoizedState = ref;
	return ref;
	}
	}

But components should not have side effects during render, so this distinction shouldn't matter.

[gmoniava]

React renders twice in strict mode, without resetting anything

Hm before you said

This means that, in DEV mode, React will render a component once- throw away any state updates- and then render again

Isn't throw away and resetting kind of similar? I thought it could throw away a ref too. Ps I think you are suggesting in the example of this thread there was update, in my question it was mount.

…

On Wed, Oct 13, 2021, 2:26 AM Brian Vaughn ***@***.***> wrote: React renders twice in strict mode, without resetting anything: https://github.com/facebook/react/blob/1e247ff892985488c6bcb87e924d7881f5c37261/packages/react-reconciler/src/ReactFiberBeginWork.new.js#L971-L1000 If it's an *update*, useRef will do *exactly the same thing*: https://github.com/facebook/react/blob/1e247ff892985488c6bcb87e924d7881f5c37261/packages/react-reconciler/src/ReactFiberHooks.new.js#L1632-L1635 If it's a *mount*, useRef will be initialized twice: https://github.com/facebook/react/blob/1e247ff892985488c6bcb87e924d7881f5c37261/packages/react-reconciler/src/ReactFiberHooks.new.js#L1560-L1630 But components should not have side effects during render, so this distinction shouldn't matter. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18003 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASNHTLRLX5L6O7Y6UGSGMQLUGSY2HANCNFSM4KR56ONA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[bvaughn]

I think this is going to be my last response. I don't think this conversation is super helpful, since you seem to be ignoring the fact that the example code you posted has side-effects and so is known/expected to be broken. (This is the purpose of StrictMode double rendering, as the link I shared above explains.)

Isn't throw away and resetting kind of similar? I thought it could throw away a ref too.

Refs are mutable values. React initializes them once and then returns the same object/wrapper every time your component renders (including strict mode).

[gmoniava]

@bvaughn Well thanks for the effort anyway. I don't want to sound rude but I also have a feeling you didn't try to understand some of my comments above.

[bvaughn]

I'm sorry you feel that that way, but I answered your question in my comment above (#18003 (comment)) when I said that if your component is mounting, React will reset the ref between regular and strict mode render. That's why you see a different behavior than your code that modifies a local variable (which is not managed by React and so is only true for the initial render).

That being said, the reason I said this is not a very useful discussion is because the code pattern you're demonstrating is known to be broken and our documentation explicitly recommends against it (for reasons like the one we've been discussing).