atlassian/react-sweet-state

Using container on update hook causes React 16 errors to be printed

yamadapc opened this issue · 5 comments

When using container onUpdate hooks and setting state in them, react-sweet-state may call set state during render when the container props change.

This causes a warning on react 16 "Warning: Cannot update a component (Child) while rendering a different component".

The problem is caused by this section running on render:

    if (!shallowEqual(propsRef.current.next, propsRef.current.prev)) {
      containedStores.forEach(({ handlers }) => {
        handlers.onContainerUpdate?.(
          propsRef.current.next,
          propsRef.current.prev
        );
      });
    }

The following example reproduces the bug:

import "./styles.css";
import React, { useState } from "react";
import { createHook, createContainer, createStore } from "react-sweet-state";

export const hydrateContainer =
  () =>
  ({ setState }, { value }) => {
    setState({
      value,
    });
  };

const Store = createStore({
  actions: {},
  initialState: { value: 0 },
});
const Container = createContainer(Store, {
  onInit: hydrateContainer,
  onUpdate: hydrateContainer,
});

const useStore = createHook(Store);

function Child() {
  const [{ value: storeValue }] = useStore();
  return <div>storeValue: {storeValue}</div>;
}

export default function App() {
  const [value, setValue] = useState(0);

  return (
    <Container value={value}>
      value: {value}
      <Child />
      <button onClick={() => setValue((v) => v + 1)}>
        Click to bug out sweet state!
      </button>
    </Container>
  );
}

A sandbox reproduction is on - https://codesandbox.io/p/sandbox/cranky-dew-lvdhwv?file=%2Fsrc%2FApp.jsx%3A1%2C1-42%2C1

Duplicated #219
We have a PR up #221 that however needs validation

Is it duplicated?

This isn't really a "change in behaviour caused by React 17 to 18", but it is a bug in React 16 where using the setState API in onUpdate will always trigger an error to be logged.

This is a problem because developers will be confused by the warnings being logged and not know that they are coming from React Sweet State.

This is still an issue with certain options after #221.

I suggest the setState parameter be removed from the update hook API on the cases it will break / print a warning.

Marked as duplicated because it has the same root cause: the conversion to functional component. By going back to class and using getDerivedStateFromProps to trigger those state changes, the warning and the deoptimisation go away (could not reproduce anymore).
I assume because that static method is called before render phase and so mutating state is still a valid operation

While we work on #221 , a workaround is to set

defaults.batchUpdates = true

which is what we use across some products in production