search

facebookexperimental / Recoil

Recoil is an experimental state management library for React apps. It provides several capabilities that are difficult to achieve with React alone, while being compatible with the newest features of React.
https://recoiljs.org/
MIT License
19.59k stars 1.18k forks source link

fix: atomRegistry deletion overlaps in the cleanup stage #2295

Open cjvnjde opened 9 months ago

cjvnjde commented 9 months ago

I found an issue particularly noticeable in strict mode: initializing two effects followed by a cleanup operation erroneously removes cached data for both effects. This issue becomes particularly apparent in environments combining strict mode with Next.js. To illustrate, consider the following sequence of events (real sequence on my project):

set key:  testState       1
delete key:  testState    0
set key:  testState       1
set key:  testState       2
delete key:  testState    0 // Issue: should be 1 because only one deletion occurred

The root cause of this issue is the reuse of the same key for multiple 'set' operations, leading to incorrect 'delete' behavior. A single cleanup call affects all instances with the same key rather than the specific instance intended.

To resolve this, we need to use a distinct key for each 'set' operation, and the corresponding 'delete' operation targets this specific key. This approach ensures accurate tracking and removal of individual effects.

Additionally, this fix also fixes this issue. The reason for that problem was the deletion of the "second key" that had been deleted with the previous cleaning.

FezVrasta commented 7 months ago

I'm experiencing https://github.com/facebookexperimental/Recoil/issues/1994 just using syncEffect while I'm trying to upgrade from React 17 to 18, on Chrome.