pmndrs / jotai

👻 Primitive and flexible state management for React
https://jotai.org
MIT License
18.3k stars 590 forks source link

fix: do not abort atoms on unmount #2627

Closed Pinpickle closed 2 months ago

Pinpickle commented 2 months ago

Related Bug Reports or Discussions

Fixes #2625

Summary

@dai-shi agreed that not aborting promises on unsubscription was the best solution here.

When added to the continuable/cancellable promise idiom, it makes quite a lot of sense. Only abort the promise if you can replace it with another one.

I added the behaviour to store and store2

Check List

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jotai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2024 5:03am
codesandbox-ci[bot] commented 2 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

github-actions[bot] commented 2 months ago

LiveCodes Preview in LiveCodes

Latest commit: fe3fad5191e2792f2e8efba254d8e869099f4d30
Last updated: Jun 27, 2024 5:03am (UTC)

Playground Link
React demo https://livecodes.io?x=id/3SENPF48Y

See documentations for usage instructions.

Pinpickle commented 2 months ago

@dai-shi

 ❯ |jotai| tests/react/abortable.test.tsx  (4 tests | 1 failed) 4208ms
   ❯ tests/react/abortable.test.tsx > abortable atom test > can abort on unmount
     → expected +0 to be 1 // Object.is equality

What do we want to do here? As far as I can tell, it's impossible to satisfy this test case.

dai-shi commented 2 months ago

can abort on unmount

Yeah, it's the test case we will break. Just remove it?

dai-shi commented 2 months ago

☝️ @Pinpickle

Pinpickle commented 2 months ago

@dai-shi sorry! I've pushed the fix now. I've kept the test to assert that there are no aborts on unmount so this behaviour doesn't get reverted :)

Pinpickle commented 2 months ago

Oops, had a TS error. Fixed now!

Pinpickle commented 2 months ago

Thanks @dai-shi!