Closed perrin4869 closed 3 years ago
I think at the time we decided that we don't actually know if it's safe to bail out in all cases until we try render again. The "bailout" here means that it doesn't go ahead to render children. But re-running the same function might be necessary (for example, if a reducer is inline and we don't know if we bail out until we re-run the reducer on next render). So for consistency we always re-run it on updates during the render phase.
I don't think I agree with you the pattern in https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops is broken. It's different from your code sample which doesn't track prevState
.
@gaearon Thanks for your response!
I elavorated on my first example here: https://codesandbox.io/s/84qo167659
I was indeed thinking about the use-case where you have a previous value, only instead of storing the previous value inside the state, I was storing it using usePrevious
hook. The problem with that approach is that the effect inside usePrevious
never has a chance to fire, updating the previous value, and stopping the infinite renders... thus requiring a check before firing setState
to check if the state actually changes, which is not very intuitive in my opinion... In the specific use-case where state does not change, maybe it shouldn't re-run the render phase?
Let’s keep it open so I remember to look into it and reply.
Thanks for looking into this!
Maybe another solution could be to introduce an "official" usePrevious
hook which does not rely on useEffect
but updates itself using React internals, even between synchronous render calls
I believe I am seeing the same issue, but with slightly different reproduction. I agree with @perrin4869 that the docs seem to be clear that updating the state to the exact ===
same value will bail out of re-rendering.
Below is an example jest spec that reproduces the issue
import { createElement, useCallback, useEffect, useState } from 'react';
import { fireEvent, render } from 'react-testing-library';
// This component is contrived but shows the behavior
function ComponentSettingState({
results
}: {
results: (value: number) => void;
}) {
const [state, setState] = useState(0);
// Call with our current state
results(state);
// A never changing callback
const handleClick = useCallback(() => setState(1), []);
// Contrived effect that sets state to the same current value.
useEffect(
() => setState((prevState) => prevState) // This still triggers a re-render after the click occurs
/* run after ever render */
);
return <button onClick={handleClick}>click</button>;
}
test('useState setting is re-rendering even if state does not change', () => {
const results = jest.fn();
const { getByText } = render(<ComponentSettingState results={results} />);
expect(results).toHaveBeenCalledTimes(1);
expect(results).toHaveBeenNthCalledWith(1, 0); // Only rendered once
fireEvent.click(getByText('click'));
expect(results).toHaveBeenCalledTimes(2); // ERROR! it was called 3 times. It rendered twice here instead of the expected once
expect(results).toHaveBeenNthCalledWith(2, 1);
// The following makes the test pass
// expect(results).toHaveBeenCalledTimes(3);
// expect(results).toHaveBeenNthCalledWith(2, 1);
// expect(results).toHaveBeenNthCalledWith(3, 1);
});
I am able to do manual strict equality checks on the state before calling setState to bypass the issue. But it seems to contradict the docs.
I want to subscribe this issue/bug, seems like React takes care of updates in theory but not in practice, React will redo calculations even if the n state and n-1 state are the same.
useReducer dispatch function also triggers
Just to clarify — you’re worrying about this because of some perf issue right? It shouldn’t affect the end result. Or does it?
I haven’t looked into this report yet.
Also to be clear the bailout isn’t supposed to prevent calling the render. It prevents rendering child components. So you shouldn’t worry about this component’s own render running again.
Does that clarify things?
Yes, it is a perf issue from my point of view, render result is the same cause the state is the same.
Then I would like to ask how can we tell React that this component does not need a render at all if state does not change? Right now, I am not calling a set function from useState or useReducer if I know it is not going to change the state (using previousState and conditions)
What exactly are you doing inside your render that is so expensive? I'd like to understand the practical issue you're running into. Usually bailing out of rendering child subtree is enough.
function BatchFunction() {
const [value, valueSet] = React.useState(0)
const onClick = React.useCallback(() => {
valueSet(1)
}, [])
console.log(`BatchFunction`, `RENDER`, { value })
return (
<div onClick={onClick}>
BatchFunction
</div>
)
}
I think the main issue here is that the usePrevious
hook given in docs didn't take render phase updates into account. It doesn't update the previous value until after render phase. But in this case, we need to read the latest previous prop immediately after an update is scheduled during render phase.
@gaearon this issue cause this problem.
If we call a setState with no change new state in a async call back, we can't get a valid previous value in a async callback to decide whether to call the setState. Because if we use a callback version of setState to get the previous value, the render always trigger. However if setState
does not trigger render like dependents of useEffect and other hooks, we still can force updating by using setState({})
or setState(preValue => preValue + 1)
or setState(preValue => !preValue).
If you have a problem, please provide a concrete runnable example showing the problem. We can't help if you provide no runnable code example (such as a sandbox). Thanks.
example This is the example. click async button first, and immediate the sync button. Randomly, console will output this:
We got a random next counter with 2. If the next counter is equal to current, we shouldn't call the setCounter, But how can we know it?
The real pre counter is 2, And the counter has not changed yet. But we still call the setCounter, because we have not a way to detect it.
Regarding example in https://codesandbox.io/s/84qo167659, the fix to this is to use render phase update to keep track of the previous value, just like in the documentation: https://codesandbox.io/s/v34o9rx5m5
I'm not sure I understand why you're trying to do things differently since I already linked to this doc section in an earlier comment. Maybe I'm missing something :-)
@vipcxj I don't understand what problem this example is demonstrating. Can you rephrase this example from a hypothetical one with counters to a more concrete one where it's actually causing problems in your app?
setState
like in the example. But I do not want to call it without knowing whether the new counter is equal to the previous one.What is the actual problem you're solving in your app? I imagine it's not actually a counter app. Explaining it from user perspective might help.
I have a big state in a function component. And I have a effect with a async callback. There are a lot of setState in it. And lots of them may not change the state. But they are async, so they will not be batched up. I have no way to skip rerender if the state is not really changed.
The async callback will last long time, the state may be changed outside. So the previous state in the async callback is not reliable, the only way to get the reliable current state is use setState(preState => XXX)
, That's what I'm trying to avoid
@gaearon I have updated the example
I added a comparison example which using class component. In the class component, I always can get the current state by using this.state
, this is impossible in function component, this is why I think the useState hook should deal with the comparision of state itself
I added a child component with render counter to each version: https://codesandbox.io/s/oxz2l25yr5
Can you please explain what sequence of steps I need to do in order to show that function children re-render more often than class children?
@gaearon I want to build a very dynamic dom tree depend on a remote config. So I do a complicate calculation of dom tree depend on the remote config in the render method (of course, the network call is in the useEffect
). Too many render in the current level may be expensive. So bailing out of rendering child subtree may be not enough to me. On the other hand, too many render is hard to debug. If I put a breakpoint in the render method, it always stop in useless situation.
You can always split a component in two and do expensive calculation in the inner one.
I have two sandboxes with essentially exactly the same code. The only difference is the second one is a fork from a @gaearon sandbox he included in https://overreacted.io/a-complete-guide-to-useeffect/, but his original code has been replaced.
What is super weird is that in one sandbox (first one), updating the state with the same value using the function returned from useState
does not cause a re-render. In the second one (fork), it causes a re-render.
Frustrating because I am trying to get to the bottom of the issue of whether or not updating the state with the same value causes a rerender, and I have essentially the same code doing different things. Maybe because they are using different versions of React?
Here are the two sandboxes:
https://codesandbox.io/s/r0vz0n13mm https://codesandbox.io/s/0o1917z8ln
@rscharfer Seems like in the second one, you've used an alpha version of 16.8.0 which might be buggy. Always go with final versions just to be safe.
@privateOmega according to @gaearon , the first one is the buggy one. setState
cause render
being called even when the arguments of setSate
not change. However the children component will not rerender when the arguments of setSate
not change. I think it is so weird, but it is the designed behaviour.
@gaearon Do you know if there is solution? Same here happens to me, it re-renders every time even if state is not changing. Thanks in advance buddy
About preventing re-renders when state does not change. I use a custom hook like this:
function areStrictEqual(a, b) {
return a === b;
}
function useStateUpdate(initialState, areEqual = areStrictEqual) {
const [state, setState] = React.useState(initialState);
const stateRef = React.useRef(state);
const areEqualRef = React.useRef(areEqual);
areEqualRef.current = areEqual;
const updateState = React.useCallback(stateOrReducer => {
const nextState =
typeof stateOrReducer === "function"
? stateOrReducer(stateRef.current)
: stateOrReducer;
if (!areEqualRef.current(stateRef.current, nextState)) {
stateRef.current = nextState;
setState(nextState);
}
}, []);
return [state, updateState];
}
Example usage: https://codesandbox.io/s/misty-frost-yiogh
@mctep That custom hook doesn't look concurrent-mode safe. An in-progress render could call updateState
which would update the stateRef
, but that render might be thrown away, leaving the stateRef
out of sync with state
.
I think the ref update needs to happen in the commit phase
function useStateUpdate(initialState) {
const [state, setState] = React.useState(initialState);
const stateRef = React.useRef(state);
React.useLayoutEffect(() => {
stateRef.current = state;
});
const updateState = React.useCallback(stateOrReducer => {
const nextState =
typeof stateOrReducer === "function"
? stateOrReducer(stateRef.current)
: stateOrReducer;
if (stateRef.current !== nextState) {
setState(nextState);
}
}, []);
return [state, updateState];
}
https://codesandbox.io/s/elated-cdn-pk37w?file=/src/index.js
Maybe the docs should be updated to make the behaviour super clear?
The 'Functional Updates' part says:
If your update function returns the exact same value as the current state, the subsequent rerender will be skipped completely
https://reactjs.org/docs/hooks-reference.html#functional-updates
Need to scroll a little further down to read the longer 'Bailing out of a state update' section which explains that only children-renders and effects are not run.
@gaearon Why we can't prevent to re-render? #18719
@mctep @accordeiro Thanks for a solution but we should know about this issue
Looking through the implementation, useState
is implemented using useReducer
, and reducers are run during a render because the function might be defined inline and access the values returned by earlier hooks. e.g.:
function Component() {
const value = useHookThatReturnsValue();
const [state, dispatch] = useReducer((state, action) => {
// inline reducer uses 'value' so is run as part of the render
// so it is sure to access the most up-to-date values
return updateState(state, action, value);
)};
return <Child state={state} dispatch={dispatch} />
}
This means that setState
also runs as part of a new render, not before.
I am presuming that setState(currentState)
does not bail out so that there are no observable differences between setState(currentState)
and setState(() => currentState)
or when changing a useState
to a useReducer
. The component will always render the same number of times. i.e. It is might be possible to change setState to bail out earlier, but then there would be subtle differences between useState
and useReducer
.
function Component() {
console.log('3. pre-useState');
const [state, setState] = React.useState(0);
console.log('5. post-useState');
if (state === 0) {
console.log('1. pre-SetState');
setState(() => {
// delayed until the Component is rendered again
console.log('4. setState');
return 1;
});
console.log('2. post-SetState');
}
return <div />;
}
logs:
3. pre-useState
5. post-useState
1. pre-SetState
2. post-SetState
3. pre-useState
4. setState
5. post-useState
https://codesandbox.io/s/patient-darkness-exvkp?file=/src/App.js
I am presuming that setState(currentState) does not bail out so that there are no observable differences between setState(currentState) and setState(() => currentState) or when changing a useState to a useReducer. The component will always render the same number of times
I don't think this is a good justification for the current behavior. You shouldn't rely on the number of renders to begin with. Even if you use the same setState
signature you could still end up with a different number of renders in concurrent mode.
I think the takeaway is that you should stop worrying about render counts until you identified a performance bottleneck. If your render function does expensive work then you can memoize that expensive work with useMemo
. That will not run again on a render that was triggered by setState(currentState)
. The same applies to useEffect
: https://codesandbox.io/s/why-render-doesnt-matter-zrqpb
stop worrying about render counts until you identified a performance bottleneck
For me it's not performance but infinite renders. It would be handy if I didn't have to worry about checking the current state before resetting it to something.
function Component(props) {
const [state, setState] = useState(0);
if (someCondition(props)) {
setState(0); // Error! Too Many ReRenders
}
return <div />;
}
Can fix here without issue, but in a more complex component, with custom hooks passing state around, this is slightly harder to achieve.
@acutmore That's a really good example why this is an issue. If we'd fix your particular issue, React wouldn't need to bailout since the usercode is already responsible for that.
I guess for now we need to advise to check if the state wouldn't change when implementing the "get-derived-state-from-props"-pattern.
Edit: In all fairness: The docs do document it with a guard against setState(prevState)
. See https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops
@acutmore Calling setState directly from a component render function is a footgun and should probably be an explicit error because sometimes it works and sometimes it doesn't. Your state change needs to be extracted to an effect hook.
function Component(props) {
const [state, setState] = useState(0);
const condition = someCondition(props);
useEffect(() => {
if (condition) {
setState(0); // This is fine
}
}, [ condition ]);
return <div />;
}
@laverdet absolutely that would avoid the too many re-render errors. Though now the child effects are potentially being run twice. (child effects run before parent effects)
https://codesandbox.io/s/busy-burnell-by65v?file=/src/App.js
Sure but now we're back to an issue of efficiency instead of an issue of correctness.
React does not offer a strong guarantee about when it invokes render functions. Render functions are assumed to be pure and there should be absolutely no difference for correctness regardless of how many times they're called. If calling it an extra time causes a bug, it's a problem with the code that needs to be fixed.
From the performance point of view, the cost of re-running a single function is usually negligible. If it's not, you should add some useMemo
s to the parts that are expensive. React does guarantee that if a bailout happens, it will not go updating child components. Even though it might in some cases re-run the component function itself.
I don't think there is something actionable here, so I think we can close this.
Do you want to request a feature or report a bug?
Bug
What is the current behavior?
As demonstrated in this codesandbox, trying to implement a pattern similar to the one discussed in https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops results in an infinite loop, even if the value of the state does not change. This seems like a bug, because, as documented here,
If you update a State Hook to the same value as the current state, React will bail out without rendering the children or firing effects.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
What is the expected behavior?
Since the state does not change, bail out on the re-render. This can be worked around by adding a check before
setState
to check if the state has changed before calling the function.Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?