jotai-recoil issue since 2.3.0+

[dai-shi]

This is a tracking issue for #2164. See discussions there.

Reproduction: https://codesandbox.io/s/flamboyant-sanderson-kp8k3q?file=/src/App.tsx

I think it's a bug in #2029 and it's not specific to jotai-recoil.

Tasks:

1. Create a failing test (without jotai-recoil, hopefully only with store api)
2. Fix it

[dai-shi]

I wonder why we didn't have such tests until now.

[dai-shi]

Here's the failing test. It passes on v2.2.3.

diff --git a/tests/vanilla/store.test.tsx b/tests/vanilla/store.test.tsx
index a95d35b..6093439 100644
--- a/tests/vanilla/store.test.tsx
+++ b/tests/vanilla/store.test.tsx
@@ -423,3 +423,15 @@ it('should update derived atoms during write (#2107)', async () => {
   store.set(countAtom, 2)
   expect(store.get(countAtom)).toBe(2)
 })
+
+it('should not recompute a derived atom value if unchanged (#2168)', async () => {
+  const store = createStore()
+  const countAtom = atom(1)
+  const derived1Atom = atom((get) => get(countAtom) * 0)
+  const derive2Fn = vi.fn((get: Getter) => get(derived1Atom))
+  const derived2Atom = atom(derive2Fn)
+  expect(store.get(derived2Atom)).toBe(0)
+  store.set(countAtom, (c) => c + 1)
+  expect(store.get(derived2Atom)).toBe(0)
+  expect(derive2Fn).toHaveBeenCalledTimes(1)
+})

[eryue0220]

Can I have a tried?

[dai-shi]

Sure!

[dai-shi]

@eryue0220 Any progress?
Should I work on it, or does anyone else want to try?

[eryue0220]

@eryue0220 Any progress? 有什么进展吗？ Should I work on it, or does anyone else want to try?我应该努力，还是其他人想尝试？

I'm still working on it. Maybe more some time.

[eryue0220]

@dai-shi Hello，I have some doubts about this. I found the bug occurred because the hasPromiseAtomValue call was omitted in this line. But I don't have much context to know why.

[dai-shi]

This line?

jotai/src/vanilla/store.ts

Line 356 in cdbfcdf

const readAtomState = <Value>(

Can you open a PR? I will see if it makes sense or not.

[eryue0220]

Sorry, I marked a wrong line. It's here, and I committed the PR, but I'm still working on it. The current code still exists bugs.

[dai-shi]

Ah, I see. It doesn't seem ideal, because it recomputes for all promise dependencies (some tests may fail, or such tests are missing.)

btw, we have another issue related with promise handling. #2192 (reply in thread)
It's not the same issue as it doesn't fix with v2.2.3, but it might be somehow related.
Hopefully, we have a single solution to cover both.

[dai-shi]

Hopefully, we have a single solution to cover both.

It's unlikely as I investigated #2199.

[dai-shi]

I think it was our intentional behavioral change in v2.3.0.

So, #2168 (comment) the test doesn't take that into account.

It should subscribe to derived2Atom.

Let me see if we can work around in jotai-recoil.

[dai-shi]

Ooops, that's my oversight again. It's not solvable on jotai-recoil...
So, I guess we should give up the idea in #2029 unless we find a better solution.

[stevemolitor]

I guess we should give up the idea in #2029 unless we find a better solution.

Does this mean #2029 will get reverted in the next Jotai release?

My current project (migrating a big app from Recoil to Jotai) is blocked in some areas. No hurries - I'm just trying to decide if we should downgrade to Jotai v2.2.3 for now, or wait for this bug to get fixed.

Happy to help in any fashion also.

Thanks!

[dai-shi]

Sorry for the confusion. I was confused too. I will merge #2197 soon, which should fix the issue. Let me try your reproduction.