reusable icon indicating copy to clipboard operation
reusable copied to clipboard

Bug: Async state updates in React 17

Open bucknermr opened this issue 3 years ago • 4 comments

When asynchronously updating the state of a reusable store multiple times, sometimes the final update is not reflected in the state that is returned to the subscriber.

This appears to only happen when updating the state an even number of times, from within an async function. The value remains stale until the state is updated again.

The reproduction below uses a contrived example with Promise.resolve, but the issue seems to occur for any async function (i.e. after fetching data, setTimeout callback, etc).

Reproduce

Sandbox reproduction: https://codesandbox.io/s/reusablejs-react-17-bug-4hg8n?file=/src/App.js

This same code seems to be working on React 16: https://codesandbox.io/s/reusablejs-react-16-no-bug-g8jz5?file=/src/App.js

Additional Info

  • package.json (from codesandbox above):
{
  "name": "reusablejs-react-17-bug",
  "version": "1.0.0",
  "description": "",
  "keywords": [],
  "main": "src/index.js",
  "dependencies": {
    "react": "17.0.2",
    "react-dom": "17.0.2",
    "react-scripts": "4.0.0",
    "reusable": "1.0.1"
  },
  "devDependencies": {
    "@babel/runtime": "7.13.8",
    "typescript": "4.1.3"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
    "eject": "react-scripts eject"
  },
  "browserslist": [
    ">0.2%",
    "not dead",
    "not ie <= 11",
    "not op_mini all"
  ]
}
  • which browser Chrome, Safari

bucknermr avatar Feb 15 '22 01:02 bucknermr

Note: The conditions for this bug are really odd, so I'm not sure if what I'm seeing is the whole issue or if there is something else going on. I stumbled this issue when trying to reproduce a different issue I've been seeing with Reusable+React 17: stale values occasionally being returned from the reusable store when using the selector function, but I've been unable to reproduce that issue.

Also wondering if this may be related https://github.com/reusablejs/reusable/issues/123

bucknermr avatar Feb 15 '22 01:02 bucknermr

I think I'm running into a similar/same issue right now with react-native:


const useDoubleCounter = createStore(() => {
    const [counterA, setCounterA] = React.useState({ value: 0 })
    const [counterB, setCounterB] = React.useState({ value: 0 })

    const increment = () => {
        const incremented = { value: counterA.value + 1 }
        console.debug("Incrementing to " + incremented.value)
        setCounterA(incremented)
        setCounterB(incremented)
    }

    console.debug({ counterA, counterB })

    return {
        counterA,
        counterB,
        increment
    }
})

const CounterView = () => {
    const { counterA, counterB, increment } = useDoubleCounter()

    return <View style={{ flexDirection: "column", alignItems: "center", flex: 1, justifyContent: "space-evenly" }}>
        <Text>{counterA.value}</Text>
        <Text>{counterB.value}</Text>
        <Button title="Increment" onPress={increment} />
    </View>
}

When I push the button, the two console.debugs print out the correct incremented value, but the result from the useDoubleCounter()-hook always laggs behind one increment (e.g. pushing the first time prints out "Incrementing to 1", but the Text-elements will continue to show a "0").

Not wrapping the states in an object makes the issue disappear for some reason.

Trying to update the states multiple times inside the increment handler with the same value (both using the same object, or a newly created object with the same value) does not hide the issue.

Involved versions of react/reusable:

[email protected]
[email protected]

CarpeNecopinum avatar Aug 31 '22 11:08 CarpeNecopinum

So one thing I noticed looking at the source code:

At https://github.com/reusablejs/reusable/blob/master/src/react-reusable.tsx#L81

const [localCopy, setLocalCopy] = useState<SelectorValue>(() => selector(store.getCachedValue()));

  useEffect(() => {
    return store.subscribe((newValue) => {
      const selectedNewValue = selector(newValue);

      if (!areEqual(selectedNewValue, localCopy)) {
        setLocalCopy(() => selectedNewValue);
      }
    });
  }, [store, localCopy, selector, areEqual]);

As useEffects are run after the render step, there is a chance of "losing" a notification between the initial state of localCopy and the store.subscribe becoming active.

Updating the subscribe method to force an explicit update (that may be redundant, but should be caught by areEqual) fixes both, my minimal example from above and a more contrived example that is part of a bigger project I'm working on:

  subscribe(callback: StoreValueChangeCallback<HookValue>) {
    this.subscribers = [...this.subscribers, callback];
    if (this.cachedValue != null) callback(this.getCachedValue())
    return () => {
      this.subscribers = this.subscribers.filter(sub => sub !== callback)
    }
  }

If desired, I can turn this into a PR

CarpeNecopinum avatar Aug 31 '22 13:08 CarpeNecopinum

I also tested that fix against @bucknermr's Sandbox and it appears to solve the issue they're having, too.

CarpeNecopinum avatar Aug 31 '22 13:08 CarpeNecopinum