From a1b1998db71d192dbc1e1691923330bae482df12 Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 15:32:42 +0330 Subject: [PATCH 01/11] fix: prevent calling storage subscribe unless in client --- src/vanilla/utils/atomWithStorage.ts | 36 +++++++++++++++------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/vanilla/utils/atomWithStorage.ts b/src/vanilla/utils/atomWithStorage.ts index b508854f37..ba56841827 100644 --- a/src/vanilla/utils/atomWithStorage.ts +++ b/src/vanilla/utils/atomWithStorage.ts @@ -161,30 +161,34 @@ export function createJSONStorage( callback(newValue) }) - let subscriber = getStringStorage()?.subscribe if ( - !subscriber && typeof window !== 'undefined' && - typeof window.addEventListener === 'function' && - window.Storage && - getStringStorage() instanceof window.Storage + typeof window.addEventListener === 'function' ) { - subscriber = (key, callback) => { - const storageEventCallback = (e: StorageEvent) => { - if (e.storageArea === getStringStorage() && e.key === key) { - callback(e.newValue) + let subscriber = getStringStorage()?.subscribe + if ( + !subscriber && + window.Storage && + getStringStorage() instanceof window.Storage + ) { + subscriber = (key, callback) => { + const storageEventCallback = (e: StorageEvent) => { + if (e.storageArea === getStringStorage() && e.key === key) { + callback(e.newValue) + } + } + window.addEventListener('storage', storageEventCallback) + return () => { + window.removeEventListener('storage', storageEventCallback) } - } - window.addEventListener('storage', storageEventCallback) - return () => { - window.removeEventListener('storage', storageEventCallback) } } - } - if (subscriber) { - storage.subscribe = createHandleSubscribe(subscriber) + if (subscriber) { + storage.subscribe = createHandleSubscribe(subscriber) + } } + return storage } From 71d2fa02c4272fe143db065cb37fa9446e12d0cf Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 16:10:10 +0330 Subject: [PATCH 02/11] test: add test for getStringStorage method from createJSONStorage to not be called in server --- tests/react/vanilla-utils/atomWithStorage.test.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 2325e1e250..0427806f62 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -9,7 +9,10 @@ import { createJSONStorage, unstable_withStorageValidator as withStorageValidator, } from 'jotai/vanilla/utils' -import type { SyncStringStorage } from 'jotai/vanilla/utils/atomWithStorage' +import type { + AsyncStringStorage, + SyncStringStorage, +} from 'jotai/vanilla/utils/atomWithStorage' const resolve: (() => void)[] = [] @@ -337,7 +340,7 @@ describe('atomWithStorage (in non-browser environment)', () => { const asyncStorageData: Record = { count: '10', } - const asyncDummyStorage = { + const asyncDummyStorage: AsyncStringStorage = { getItem: async (key: string) => { await new Promise((r) => resolve.push(r)) return asyncStorageData[key] as string @@ -367,6 +370,13 @@ describe('atomWithStorage (in non-browser environment)', () => { expect(storage.subscribe).toBeUndefined() }) + + it('createJSONStorage with custom subscribe', async () => { + const subscriber = vi.fn() + createJSONStorage(subscriber) + + expect(subscriber).not.toBeCalled() + }) }) describe('atomWithStorage (with browser storage)', () => { From 6ca781f7e95c61c4b3d6f8b1d5478bd0d64526a7 Mon Sep 17 00:00:00 2001 From: daishi Date: Wed, 29 May 2024 21:55:55 +0900 Subject: [PATCH 03/11] use try-catch --- src/vanilla/utils/atomWithStorage.ts | 43 +++++++++++++++------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/vanilla/utils/atomWithStorage.ts b/src/vanilla/utils/atomWithStorage.ts index ba56841827..150ae5b9cb 100644 --- a/src/vanilla/utils/atomWithStorage.ts +++ b/src/vanilla/utils/atomWithStorage.ts @@ -161,34 +161,37 @@ export function createJSONStorage( callback(newValue) }) + let subscriber: StringSubscribe | undefined + try { + subscriber = getStringStorage()?.subscribe + } catch { + // ignore + } if ( + !subscriber && typeof window !== 'undefined' && - typeof window.addEventListener === 'function' + typeof window.addEventListener === 'function' && + window.Storage ) { - let subscriber = getStringStorage()?.subscribe - if ( - !subscriber && - window.Storage && - getStringStorage() instanceof window.Storage - ) { - subscriber = (key, callback) => { - const storageEventCallback = (e: StorageEvent) => { - if (e.storageArea === getStringStorage() && e.key === key) { - callback(e.newValue) - } - } - window.addEventListener('storage', storageEventCallback) - return () => { - window.removeEventListener('storage', storageEventCallback) + subscriber = (key, callback) => { + if (!(getStringStorage() instanceof window.Storage)) { + return () => {} + } + const storageEventCallback = (e: StorageEvent) => { + if (e.storageArea === getStringStorage() && e.key === key) { + callback(e.newValue) } } - } - - if (subscriber) { - storage.subscribe = createHandleSubscribe(subscriber) + window.addEventListener('storage', storageEventCallback) + return () => { + window.removeEventListener('storage', storageEventCallback) + } } } + if (subscriber) { + storage.subscribe = createHandleSubscribe(subscriber) + } return storage } From 2838849e6b5a94b9ebc69e8255dff88caab53ec0 Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 16:45:11 +0330 Subject: [PATCH 04/11] Revert "test: add test for getStringStorage method from createJSONStorage to not be called in server" This reverts commit 71d2fa02c4272fe143db065cb37fa9446e12d0cf. --- tests/react/vanilla-utils/atomWithStorage.test.tsx | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 0427806f62..2325e1e250 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -9,10 +9,7 @@ import { createJSONStorage, unstable_withStorageValidator as withStorageValidator, } from 'jotai/vanilla/utils' -import type { - AsyncStringStorage, - SyncStringStorage, -} from 'jotai/vanilla/utils/atomWithStorage' +import type { SyncStringStorage } from 'jotai/vanilla/utils/atomWithStorage' const resolve: (() => void)[] = [] @@ -340,7 +337,7 @@ describe('atomWithStorage (in non-browser environment)', () => { const asyncStorageData: Record = { count: '10', } - const asyncDummyStorage: AsyncStringStorage = { + const asyncDummyStorage = { getItem: async (key: string) => { await new Promise((r) => resolve.push(r)) return asyncStorageData[key] as string @@ -370,13 +367,6 @@ describe('atomWithStorage (in non-browser environment)', () => { expect(storage.subscribe).toBeUndefined() }) - - it('createJSONStorage with custom subscribe', async () => { - const subscriber = vi.fn() - createJSONStorage(subscriber) - - expect(subscriber).not.toBeCalled() - }) }) describe('atomWithStorage (with browser storage)', () => { From f38edf13301dd78ba66b512efed2e29a71ef6a31 Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 17:55:58 +0330 Subject: [PATCH 05/11] test: checking localStorage/sessionStorage access with createJSONStorage in server --- .../vanilla-utils/atomWithStorage.test.tsx | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 2325e1e250..cdab62e48f 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -353,6 +353,11 @@ describe('atomWithStorage (in non-browser environment)', () => { } const addEventListener = window.addEventListener + const localStorage = window.localStorage + const sessionStorage = window.sessionStorage + + const localStorageSpy = vi.spyOn(window.localStorage, 'getItem') + const sessionStorageSpy = vi.spyOn(window.sessionStorage, 'getItem') beforeAll(() => { ;(window as any).addEventListener = undefined @@ -360,13 +365,27 @@ describe('atomWithStorage (in non-browser environment)', () => { afterAll(() => { window.addEventListener = addEventListener + window.localStorage = localStorage + window.sessionStorage = sessionStorage + + localStorageSpy.mockRestore() + sessionStorageSpy.mockRestore() }) it('createJSONStorage with undefined window.addEventListener', async () => { const storage = createJSONStorage(() => asyncDummyStorage) - expect(storage.subscribe).toBeUndefined() }) + + it('createJSONStorage with localStorageSpy', async () => { + createJSONStorage(() => localStorage) + expect(localStorageSpy).not.toBeCalled() + }) + + it('createJSONStorage with sessionStorageSpy', async () => { + createJSONStorage(() => localStorage) + expect(sessionStorageSpy).not.toBeCalled() + }) }) describe('atomWithStorage (with browser storage)', () => { From 4abe2b5b6a451919e8c75fbed152256e3bd9deab Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 18:12:16 +0330 Subject: [PATCH 06/11] test: improve checking localStorage/sessionStorage access with createJSONStorage in server --- .../vanilla-utils/atomWithStorage.test.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index cdab62e48f..f1462149ad 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -356,20 +356,20 @@ describe('atomWithStorage (in non-browser environment)', () => { const localStorage = window.localStorage const sessionStorage = window.sessionStorage - const localStorageSpy = vi.spyOn(window.localStorage, 'getItem') - const sessionStorageSpy = vi.spyOn(window.sessionStorage, 'getItem') + const localStorageSpy = { + getItem: vi.fn(), + } beforeAll(() => { ;(window as any).addEventListener = undefined + ;(window as any).localStorage = localStorageSpy + ;(window as any).sessionStorage = localStorageSpy }) afterAll(() => { window.addEventListener = addEventListener window.localStorage = localStorage window.sessionStorage = sessionStorage - - localStorageSpy.mockRestore() - sessionStorageSpy.mockRestore() }) it('createJSONStorage with undefined window.addEventListener', async () => { @@ -377,14 +377,14 @@ describe('atomWithStorage (in non-browser environment)', () => { expect(storage.subscribe).toBeUndefined() }) - it('createJSONStorage with localStorageSpy', async () => { + it('createJSONStorage with localStorage Spy', async () => { createJSONStorage(() => localStorage) - expect(localStorageSpy).not.toBeCalled() + expect(window.localStorage.getItem).not.toBeCalled() }) - it('createJSONStorage with sessionStorageSpy', async () => { - createJSONStorage(() => localStorage) - expect(sessionStorageSpy).not.toBeCalled() + it('createJSONStorage with sessionStorage Spy', async () => { + createJSONStorage(() => sessionStorage) + expect(window.sessionStorage.getItem).not.toBeCalled() }) }) From 56d186a04db2e276b1534231c72c89ca4c54e292 Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 19:22:15 +0330 Subject: [PATCH 07/11] test: use defineProperty for checking localStorage/sessionStorage access with createJSONStorage in server --- .../vanilla-utils/atomWithStorage.test.tsx | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index f1462149ad..447ef85e10 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -356,14 +356,24 @@ describe('atomWithStorage (in non-browser environment)', () => { const localStorage = window.localStorage const sessionStorage = window.sessionStorage - const localStorageSpy = { - getItem: vi.fn(), - } + const mockGetItem = vi.fn() beforeAll(() => { ;(window as any).addEventListener = undefined - ;(window as any).localStorage = localStorageSpy - ;(window as any).sessionStorage = localStorageSpy + Object.defineProperty(window, 'localStorage', { + get() { + return { + getItem: mockGetItem, + } + }, + }) + Object.defineProperty(window, 'sessionStorage', { + get() { + return { + getItem: mockGetItem, + } + }, + }) }) afterAll(() => { @@ -378,12 +388,12 @@ describe('atomWithStorage (in non-browser environment)', () => { }) it('createJSONStorage with localStorage Spy', async () => { - createJSONStorage(() => localStorage) - expect(window.localStorage.getItem).not.toBeCalled() + createJSONStorage(() => window.localStorage) + expect(mockGetItem).not.toBeCalled() }) it('createJSONStorage with sessionStorage Spy', async () => { - createJSONStorage(() => sessionStorage) + createJSONStorage(() => window.sessionStorage) expect(window.sessionStorage.getItem).not.toBeCalled() }) }) From f1819384d20a90131e407ed6df423f4803499f2f Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Wed, 29 May 2024 19:31:36 +0330 Subject: [PATCH 08/11] fix: ts error in new test --- 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 447ef85e10..34f8d0a062 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -378,8 +378,16 @@ describe('atomWithStorage (in non-browser environment)', () => { afterAll(() => { window.addEventListener = addEventListener - window.localStorage = localStorage - window.sessionStorage = sessionStorage + Object.defineProperty(window, 'localStorage', { + get() { + return localStorage + }, + }) + Object.defineProperty(window, 'sessionStorage', { + get() { + return sessionStorage + }, + }) }) it('createJSONStorage with undefined window.addEventListener', async () => { From a864f5db99a41ac5a68abddecf70919c7e643ef5 Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Thu, 30 May 2024 12:01:23 +0330 Subject: [PATCH 09/11] improve detecting browser storage access in client --- src/vanilla/utils/atomWithStorage.ts | 1 + .../vanilla-utils/atomWithStorage.test.tsx | 20 ++++++------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/vanilla/utils/atomWithStorage.ts b/src/vanilla/utils/atomWithStorage.ts index 150ae5b9cb..01ba87508d 100644 --- a/src/vanilla/utils/atomWithStorage.ts +++ b/src/vanilla/utils/atomWithStorage.ts @@ -164,6 +164,7 @@ export function createJSONStorage( let subscriber: StringSubscribe | undefined try { subscriber = getStringStorage()?.subscribe + // getStringStorage() //?.getItem('') } catch { // ignore } diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 34f8d0a062..1715377831 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -356,22 +356,16 @@ describe('atomWithStorage (in non-browser environment)', () => { const localStorage = window.localStorage const sessionStorage = window.sessionStorage - const mockGetItem = vi.fn() - beforeAll(() => { ;(window as any).addEventListener = undefined Object.defineProperty(window, 'localStorage', { get() { - return { - getItem: mockGetItem, - } + throw new Error('localStorage is not available.') }, }) Object.defineProperty(window, 'sessionStorage', { get() { - return { - getItem: mockGetItem, - } + throw new Error('localStorage is not available.') }, }) }) @@ -395,14 +389,12 @@ describe('atomWithStorage (in non-browser environment)', () => { expect(storage.subscribe).toBeUndefined() }) - it('createJSONStorage with localStorage Spy', async () => { - createJSONStorage(() => window.localStorage) - expect(mockGetItem).not.toBeCalled() + it('createJSONStorage with localStorage', async () => { + expect(() => createJSONStorage(() => window.localStorage)).not.toThrow() }) - it('createJSONStorage with sessionStorage Spy', async () => { - createJSONStorage(() => window.sessionStorage) - expect(window.sessionStorage.getItem).not.toBeCalled() + it('createJSONStorage with sessionStorage', async () => { + expect(() => createJSONStorage(() => window.sessionStorage)).not.toThrow() }) }) From 63be0797a6311fcb8d33b2c7dd5e92cd0d6aaefe Mon Sep 17 00:00:00 2001 From: Daishi Kato Date: Fri, 31 May 2024 06:49:21 +0900 Subject: [PATCH 10/11] Update src/vanilla/utils/atomWithStorage.ts --- src/vanilla/utils/atomWithStorage.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vanilla/utils/atomWithStorage.ts b/src/vanilla/utils/atomWithStorage.ts index 01ba87508d..150ae5b9cb 100644 --- a/src/vanilla/utils/atomWithStorage.ts +++ b/src/vanilla/utils/atomWithStorage.ts @@ -164,7 +164,6 @@ export function createJSONStorage( let subscriber: StringSubscribe | undefined try { subscriber = getStringStorage()?.subscribe - // getStringStorage() //?.getItem('') } catch { // ignore } From 572815894c7f8ab741df9545a258546358c96ea6 Mon Sep 17 00:00:00 2001 From: "Mohammad H. Sattarian" Date: Mon, 3 Jun 2024 12:15:10 +0330 Subject: [PATCH 11/11] patch console.warn for atomWithStorage (in non-browser environment) test --- .../vanilla-utils/atomWithStorage.test.tsx | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/tests/react/vanilla-utils/atomWithStorage.test.tsx b/tests/react/vanilla-utils/atomWithStorage.test.tsx index 1715377831..710a2768c3 100644 --- a/tests/react/vanilla-utils/atomWithStorage.test.tsx +++ b/tests/react/vanilla-utils/atomWithStorage.test.tsx @@ -355,31 +355,43 @@ describe('atomWithStorage (in non-browser environment)', () => { const addEventListener = window.addEventListener const localStorage = window.localStorage const sessionStorage = window.sessionStorage + const consoleWarn = window.console.warn beforeAll(() => { ;(window as any).addEventListener = undefined - Object.defineProperty(window, 'localStorage', { - get() { - throw new Error('localStorage is not available.') - }, + // patch console.warn to prevent logging along test results + Object.defineProperty(window.console, 'warn', { + value: () => {}, }) - Object.defineProperty(window, 'sessionStorage', { - get() { - throw new Error('localStorage is not available.') + Object.defineProperties(window, { + localStorage: { + get() { + throw new Error('localStorage is not available.') + }, + }, + sessionStorage: { + get() { + throw new Error('sessionStorage is not available.') + }, }, }) }) afterAll(() => { window.addEventListener = addEventListener - Object.defineProperty(window, 'localStorage', { - get() { - return localStorage - }, + Object.defineProperty(window.console, 'warn', { + value: consoleWarn, }) - Object.defineProperty(window, 'sessionStorage', { - get() { - return sessionStorage + Object.defineProperties(window, { + localStorage: { + get() { + return localStorage + }, + }, + sessionStorage: { + get() { + return sessionStorage + }, }, }) }) @@ -390,6 +402,7 @@ describe('atomWithStorage (in non-browser environment)', () => { }) it('createJSONStorage with localStorage', async () => { + expect(() => createJSONStorage()).not.toThrow() expect(() => createJSONStorage(() => window.localStorage)).not.toThrow() })