facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.69k stars 46.81k forks source link

Question: shouldComponenteUpdate equivalent for state change detection in hooks #16512

Closed neilyoung closed 5 years ago

neilyoung commented 5 years ago

I'm having a component, which is facing a lot of "unnecessary" renders because it it located at a relatively low level in the component stack. Up to now I was using shouldComponentUpdate comparing internal state with the given nextState in order to determine, if it is OK to re-render. I'm wondering now, how this could be done with hooks. AFAIK memo is just covering the props changes, but what about state change comparisons?

kunukn commented 5 years ago

From the document. https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-shouldcomponentupdate

It doesn't seems to be supported for the hooks API design.

React.memo doesn’t compare state because there is no single state object to compare.

But we can apply a pattern to control when React should call render. Using useState and useReducer will trigger a render. We could use something else to store the state and use forceUpdate when we want to trigger a render update.

In this example, the render will be called when the items length is even. I have used useRef to store the state.

I am simulating that the Orders is expensive to render and I only want to update the render when enough items has been added since last time.

const Orders = () => {
  const initialState = { items: [1] };
  const [__, forceUpdate] = React.useReducer(_ => _ + 1, 0);
  const state = React.useRef(initialState).current;

  const shouldComponentReRender = update => {
    // state is prevState
    update();
    // state is nextState

    if (state.items.length % 2 === 0) forceUpdate();
  };

  const updateState = update => {
    shouldComponentReRender(update);
  };

  const onClick = () => {
    updateState(() => state.items.push(state.items.length + 1));
  };

  return (
    <div>
      <button onClick={onClick}>click</button>
      {state.items.map(item => (
        <span key={item}>{item} </span>
      ))}
    </div>
  );
};

https://codesandbox.io/s/react-hooks-state-shouldcomponentupdate-ykxzd

neilyoung commented 5 years ago

Compared to the clean shouldComponentUpdate(nextProps, nextState) this seems to be a bit overReacted :) Nevertheless, thanks.

By using useRef for the state it is possible to circumvent the ordinary render mechanism? Didn't know that, but sounds understandable.

I think the state related part of shouldComponentUpdate cannot be covered as easy as it was by using hooks and is more or less a design flaw of hooks, IMHO.

otakustay commented 5 years ago

By wrapping @kunukn 's code to a useControllableState can make it clean and easy to use:

const useForceUpdate = () => {
    const pair = useReducer(v => v + 1, 0);
    return pair[1];
};

const useControllableState = (initialState, shouldUpdate) => {
    const state = useRef(initialState)
    const forceUpdate = useForceUpdate();
    const setState = useCallback(
        newState => {
            const shouldRenderNext = shouldUpdate(state.current, newState);
            state.current = newState;
            if (shouldRenderNext) {
                forceUpdate();
            }
        },
        [forceUpdate, shouldUpdate]
    );
    return [state.current, setState];
};
neilyoung commented 5 years ago

Really? Maybe I'm too old for that. I would need more comments than code to be able to get that two weeks later :)) BTW: I already found the original code clean and nice enough, but simply not simple enough, having the primary task of comparing old state vs new state and derive a render decision from that. Even though it is more or less just a crutch...

neilyoung commented 5 years ago

And if allowed an additional question to @kunukn: Your code allows for explicit control of when to render. But how do you compare old state vs. new state in order to compare the need to render? As far as I can see you are now just deciding it no the current state.

EDIT: Disregard please. In your solution there is no need to compare old state vs. new state, since in the moment you are altering it it is changed :) Sorry.

I think I should reveal more about my current solution.

Basically it is pretty simple: It is a configuration component, so a form with several user inputs. This form is located pretty low in a bootstrap grid/panel/header thing, so it gets a lot of unrelated render requests, e.g. originated by timer functions in outer components. One of these timer periodically updates a property object named "config" from disk. So this is one source of changes. My app is a local dashboard app hosted by a Java app. So the user can change a config file manually and alter values. The parent component updates them clocked by the timer and alters the config property of my component. The configuration component reflects these changes then.

Aside of this there is a form which the user can edit. You can imagine, that it could be a very bad experience if you enter some new values and those are updated by the current values, so the manual changes are reverted magically.

For that I have up to now my "shouldComponentUpdate", which checks the entire state object (which is fed by outer properties and inner form inputs) and decides, whether it is OK to render or not.

That works absolutely fine and now I'm looking for a way to picture the same behaviour with hooks. To no avail so far...

I could circumvent by doing the disk read from the configuration component, but do I really have to refactor it that way?

I mean shouldComponentUpdate based on state comparisons SHOULD be covered by hooks as well, IMHO, otherwise it is not a full step forward preserving the past.

kunukn commented 5 years ago

If I were the one to refactor a codebase depending on state and shouldComponentUpdate to React hooks. Then I would probably lift the state up or use Context API.

And then I would be able to use React.memo and decide whether to re-render based on prevProps and nextProps.

Using the hooks API alternatively is probably a last resource to be used depending on the circumstance, code-base and React hooks experience.

neilyoung commented 5 years ago

If I were the one to refactor a codebase depending on state and shouldComponentUpdate to React hooks. Then I would probably lift the state up or use Context API.

I don't understand what that means, sorry.

I always tried to use memo It reduced the unnecessary renders to one per 10 seconds, which is exactly the rate at which the outer component retrieves the config from file. The shallow compare of memo is not detecting, that there is no change in no property of the config object.

neilyoung commented 5 years ago

I think I've found a nice solution, maybe you share your opinions on that:

const Configuration = memo(props => {

    const [config, setConfig] = useState(props.config)

    useEffect(() => {
        setConfig(props.config)
    }, [props.config])

}, (prevProps, newProps) => { return JSON.stringify(prevProps.config) === JSON.stringify(newProps.config)})

Intentionally I'm updating my internal config initially from the props and whenever it changes. via props. Additionally I'm using memo with a dumb custom property comparator in order to decide, if a render is necessary or not.

So now, in 99% of the cases, in which the props.config update contains the same internal values, no render is triggered and I can edit the values through the form. In case the file has been changed outside, the render happens (as intended).

Works for me.

threepointone commented 5 years ago

Closing since you have a workaround and this isn't a bug per se. Feel free to carry on the conversation.