From 8bd01464cf8b54def5007362bb97d419e66fa8f4 Mon Sep 17 00:00:00 2001 From: daishi Date: Tue, 14 Jan 2025 13:39:36 +0900 Subject: [PATCH 1/7] add a failing test --- tests/react/async2.test.tsx | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/react/async2.test.tsx b/tests/react/async2.test.tsx index 2796d3a260..97d992eede 100644 --- a/tests/react/async2.test.tsx +++ b/tests/react/async2.test.tsx @@ -380,3 +380,35 @@ describe('write to async atom twice', async () => { await screen.findByText('count: 4') }) }) + +describe('with onMount', () => { + it('does not infinite loop with setting a promise (#2931)', async () => { + const asyncAtom = atom(Promise.resolve(1)) + asyncAtom.onMount = (setCount) => { + setCount((p) => p.then((c) => c + 1)) + } + const Component = () => { + const [count, setCount] = useAtom(asyncAtom) + return ( + <> +
count: {count}
+ + + ) + } + await act(async () => { + render( + + + + + , + ) + }) + await screen.findByText('count: 2') + await userEvent.click(screen.getByText('button')) + await screen.findByText('count: 3') + }) +}) From 4615df70f1b180ce2ff7bb583c5353ddc9c3b7a7 Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 15 Jan 2025 10:42:28 +0900 Subject: [PATCH 2/7] a workaround --- tests/react/async2.test.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/react/async2.test.tsx b/tests/react/async2.test.tsx index 97d992eede..de8ccecf86 100644 --- a/tests/react/async2.test.tsx +++ b/tests/react/async2.test.tsx @@ -385,14 +385,17 @@ describe('with onMount', () => { it('does not infinite loop with setting a promise (#2931)', async () => { const asyncAtom = atom(Promise.resolve(1)) asyncAtom.onMount = (setCount) => { - setCount((p) => p.then((c) => c + 1)) + // We need to delay setting a new promise to avoid infinite loop + setTimeout(() => { + setCount(Promise.resolve(2)) + }) } const Component = () => { const [count, setCount] = useAtom(asyncAtom) return ( <>
count: {count}
- From 031cfaa5375924c232c53e94966e6db3973d63da Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 15 Jan 2025 10:50:47 +0900 Subject: [PATCH 3/7] add another failing test --- .../vanilla-utils/atomWithStorage.test.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index b705328e43..0d515da158 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -745,3 +745,41 @@ describe('with subscribe method in string storage', () => { // expect(storageData.count).toBe('11') }) }) + +describe('with custom async storage', () => { + it('does not infinite loop (#2931)', async () => { + let storedValue = 0 + const counterAtom = atomWithStorage('counter', 0, { + async getItem(_key: string, _initialValue: number) { + return await Promise.resolve(storedValue) + }, + async setItem(_key, newValue) { + storedValue = await new Promise((resolve) => resolve(newValue)) + }, + async removeItem() {}, + }) + const Component = () => { + const [count, setCount] = useAtom(counterAtom) + return ( + <> +
count: {count}
+ + + ) + } + await act(async () => { + render( + + + + + , + ) + }) + await screen.findByText('count: 0') + await userEvent.click(screen.getByText('button')) + await screen.findByText('count: 1') + }) +}) From 1a9d37061ddb50cdb3a876b3a4772ef960a959dc Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 15 Jan 2025 10:58:59 +0900 Subject: [PATCH 4/7] fix with cached promise --- tests/react/vanilla-utils/atomWithStorage.test.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 0d515da158..77779ee80f 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -749,9 +749,17 @@ describe('with subscribe method in string storage', () => { describe('with custom async storage', () => { it('does not infinite loop (#2931)', async () => { let storedValue = 0 + let cachedPromise: + | [typeof storedValue, Promise] + | null = null const counterAtom = atomWithStorage('counter', 0, { - async getItem(_key: string, _initialValue: number) { - return await Promise.resolve(storedValue) + getItem(_key: string, _initialValue: number) { + if (cachedPromise && cachedPromise[0] === storedValue) { + return cachedPromise[1] + } + const promise = Promise.resolve(storedValue) + cachedPromise = [storedValue, promise] + return promise }, async setItem(_key, newValue) { storedValue = await new Promise((resolve) => resolve(newValue)) From a4d1c7a80bc22e73758057fbf7b87a0c17597138 Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 15 Jan 2025 11:14:40 +0900 Subject: [PATCH 5/7] skip a test expect --- tests/react/async2.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/react/async2.test.tsx b/tests/react/async2.test.tsx index de8ccecf86..051ee1780d 100644 --- a/tests/react/async2.test.tsx +++ b/tests/react/async2.test.tsx @@ -383,12 +383,11 @@ describe('write to async atom twice', async () => { describe('with onMount', () => { it('does not infinite loop with setting a promise (#2931)', async () => { - const asyncAtom = atom(Promise.resolve(1)) + const firstPromise = Promise.resolve(1) + const secondPromise = Promise.resolve(2) + const asyncAtom = atom(firstPromise) asyncAtom.onMount = (setCount) => { - // We need to delay setting a new promise to avoid infinite loop - setTimeout(() => { - setCount(Promise.resolve(2)) - }) + setCount(secondPromise) } const Component = () => { const [count, setCount] = useAtom(asyncAtom) @@ -412,6 +411,7 @@ describe('with onMount', () => { }) await screen.findByText('count: 2') await userEvent.click(screen.getByText('button')) - await screen.findByText('count: 3') + // FIXME this fails with the current test environment. + //await screen.findByText('count: 3') }) }) From b715d53d868ae320d5f742584620ddee92170e97 Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 15 Jan 2025 11:30:25 +0900 Subject: [PATCH 6/7] fix the test --- tests/react/async2.test.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/react/async2.test.tsx b/tests/react/async2.test.tsx index 051ee1780d..f68ed4e10c 100644 --- a/tests/react/async2.test.tsx +++ b/tests/react/async2.test.tsx @@ -387,7 +387,7 @@ describe('with onMount', () => { const secondPromise = Promise.resolve(2) const asyncAtom = atom(firstPromise) asyncAtom.onMount = (setCount) => { - setCount(secondPromise) + setCount((prev) => (prev === firstPromise ? secondPromise : prev)) } const Component = () => { const [count, setCount] = useAtom(asyncAtom) @@ -411,7 +411,6 @@ describe('with onMount', () => { }) await screen.findByText('count: 2') await userEvent.click(screen.getByText('button')) - // FIXME this fails with the current test environment. - //await screen.findByText('count: 3') + await screen.findByText('count: 3') }) }) From 79d52145c42c946b0dd9323568a8170abfef1a86 Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 15 Jan 2025 11:32:32 +0900 Subject: [PATCH 7/7] tweak a test --- tests/react/vanilla-utils/atomWithStorage.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 77779ee80f..5a58323513 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -789,5 +789,7 @@ describe('with custom async storage', () => { await screen.findByText('count: 0') await userEvent.click(screen.getByText('button')) await screen.findByText('count: 1') + await userEvent.click(screen.getByText('button')) + await screen.findByText('count: 2') }) })