Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: sync useLocalStorage across components and tabs #977

Merged
merged 17 commits into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/components/create-safe/status/CreationStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ import { useCallback } from 'react'
import { useSafeCreation } from '@/components/create-safe/status/useSafeCreation'
import type { StepRenderProps } from '@/components/tx/TxStepper/useTxStepper'
import type { PendingSafeData } from '@/components/create-safe/types.d'
import useLocalStorage from '@/services/local-storage/useLocalStorage'
import StatusMessage from '@/components/create-safe/status/StatusMessage'
import useWallet from '@/hooks/wallets/useWallet'
import useIsWrongChain from '@/hooks/useIsWrongChain'
import useStatus from '@/components/create-safe/status/useStatus'

export const SAFE_PENDING_CREATION_STORAGE_KEY = 'pendingSafe'
import usePendingCreation from '@/components/create-safe/usePendingCreation'

type Props = {
params: PendingSafeData
Expand All @@ -29,10 +27,7 @@ type Props = {

export const CreationStatus = ({ params, setStep }: Props) => {
const [status, setStatus] = useStatus()
const [pendingSafe, setPendingSafe] = useLocalStorage<PendingSafeData | undefined>(
SAFE_PENDING_CREATION_STORAGE_KEY,
params,
)
const [pendingSafe = params, setPendingSafe] = usePendingCreation()
const wallet = useWallet()
const isWrongChain = useIsWrongChain()
const isConnected = wallet && !isWrongChain
Expand Down
6 changes: 2 additions & 4 deletions src/components/create-safe/steps/ReviewStep.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import { getFallbackHandlerContractInstance } from '@/services/contracts/safeCon
import { computeNewSafeAddress } from '@/components/create-safe/logic'
import { useWeb3 } from '@/hooks/wallets/web3'
import useWallet from '@/hooks/wallets/useWallet'
import useLocalStorage from '@/services/local-storage/useLocalStorage'
import type { PendingSafeData } from '@/components/create-safe/types'
import { SAFE_PENDING_CREATION_STORAGE_KEY } from '@/components/create-safe/status/CreationStatus'
import usePendingCreation from '../usePendingCreation'

type Props = {
params: SafeFormData
Expand All @@ -30,7 +28,7 @@ const ReviewStep = ({ params, onSubmit, setStep, onBack }: Props) => {
const provider = useWeb3()
const chain = useCurrentChain()
const saltNonce = useMemo(() => Date.now(), [])
const [_, setPendingSafe] = useLocalStorage<PendingSafeData | undefined>(SAFE_PENDING_CREATION_STORAGE_KEY, undefined)
const [_, setPendingSafe] = usePendingCreation()

const { maxFeePerGas, maxPriorityFeePerGas } = useGasPrice()

Expand Down
10 changes: 10 additions & 0 deletions src/components/create-safe/usePendingCreation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import useLocalStorage from '@/services/local-storage/useLocalStorage'
import type { PendingSafeData } from './types'

const SAFE_PENDING_CREATION_STORAGE_KEY = 'pendingSafe'

const usePendingCreation = () => {
return useLocalStorage<PendingSafeData | undefined>(SAFE_PENDING_CREATION_STORAGE_KEY)
}

export default usePendingCreation
8 changes: 3 additions & 5 deletions src/components/create-safe/useSetCreationStep.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { useEffect } from 'react'
import type { StepRenderProps } from '@/components/tx/TxStepper/useTxStepper'
import useIsWrongChain from '@/hooks/useIsWrongChain'
import useWallet from '@/hooks/wallets/useWallet'
import { useEffect } from 'react'
import useLocalStorage from '@/services/local-storage/useLocalStorage'
import { SAFE_PENDING_CREATION_STORAGE_KEY } from '@/components/create-safe/status/CreationStatus'
import type { PendingSafeData } from '@/components/create-safe/types.d'
import usePendingCreation from './usePendingCreation'

const useSetCreationStep = (setStep: StepRenderProps['setStep']) => {
const [pendingSafe] = useLocalStorage<PendingSafeData | undefined>(SAFE_PENDING_CREATION_STORAGE_KEY, undefined)
const [pendingSafe] = usePendingCreation()
const wallet = useWallet()
const isWrongChain = useIsWrongChain()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const useSafeAppsInfoModal = ({
} => {
const didMount = useRef(false)
const chainId = useChainId()
const [modalInfo, setModalInfo] = useLocalStorage<ModalInfoProps>(SAFE_APPS_INFO_MODAL, {})
const [modalInfo = {}, setModalInfo] = useLocalStorage<ModalInfoProps>(SAFE_APPS_INFO_MODAL)

useEffect(() => {
if (!didMount.current) {
Expand Down
2 changes: 1 addition & 1 deletion src/components/sidebar/DebugToggle/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const DebugToggle = (): ReactElement => {
const dispatch = useAppDispatch()
const isDarkMode = useDarkMode()

const [isProdGateway, setIsProdGateway] = useLocalStorage<boolean>(LS_KEY, false)
const [isProdGateway = false, setIsProdGateway] = useLocalStorage<boolean>(LS_KEY)

const onToggle = (event: ChangeEvent<HTMLInputElement>) => {
setIsProdGateway(event.target.checked)
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/safe-apps/permissions/useBrowserPermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type UseBrowserPermissionsReturnType = {
}

const useBrowserPermissions = (): UseBrowserPermissionsReturnType => {
const [permissions, setPermissions] = useLocalStorage<BrowserPermissions>(BROWSER_PERMISSIONS, {})
const [permissions = {}, setPermissions] = useLocalStorage<BrowserPermissions>(BROWSER_PERMISSIONS)

const getPermissions = useCallback(
(origin: string) => {
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/safe-apps/permissions/useSafePermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type UseSafePermissionsReturnType = {
}

const useSafePermissions = (): UseSafePermissionsReturnType => {
const [permissions, setPermissions] = useLocalStorage<SafePermissions>(SAFE_PERMISSIONS, {})
const [permissions = {}, setPermissions] = useLocalStorage<SafePermissions>(SAFE_PERMISSIONS)

const [permissionsRequest, setPermissionsRequest] = useState<SafePermissionsRequest | undefined>()

Expand Down
6 changes: 3 additions & 3 deletions src/hooks/useOwnedSafes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type OwnedSafesCache = {
const useOwnedSafes = (): OwnedSafesCache['walletAddress'] => {
const chainId = useChainId()
const { address: walletAddress } = useWallet() || {}
const [ownedSafesCache, setOwnedSafesCache] = useLocalStorage<OwnedSafesCache>(CACHE_KEY, {})
const [ownedSafesCache, setOwnedSafesCache] = useLocalStorage<OwnedSafesCache>(CACHE_KEY)

const [ownedSafes, error] = useAsync<OwnedSafes>(() => {
if (!chainId || !walletAddress) return
Expand All @@ -31,7 +31,7 @@ const useOwnedSafes = (): OwnedSafesCache['walletAddress'] => {
setOwnedSafesCache((prev) => ({
...prev,
[walletAddress]: {
...(prev[walletAddress] || {}),
...(prev?.[walletAddress] || {}),
[chainId]: ownedSafes.safes,
},
}))
Expand All @@ -43,7 +43,7 @@ const useOwnedSafes = (): OwnedSafesCache['walletAddress'] => {
}
}, [error])

return ownedSafesCache[walletAddress || ''] ?? {}
return ownedSafesCache?.[walletAddress || ''] ?? {}
}

export default useOwnedSafes
13 changes: 9 additions & 4 deletions src/services/local-storage/Storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,33 @@ class Storage {
return `${this.prefix}${key}`
}

public getItem = <T>(key: string): T | undefined => {
public getItem = <T>(key: string): T | null => {
const fullKey = this.getPrefixedKey(key)
let saved: string | null = null
try {
saved = this.storage?.getItem(fullKey) || null
saved = this.storage?.getItem(fullKey) ?? null
} catch (err) {
logError(Errors._700, `key ${key} – ${(err as Error).message}`)
}

if (!saved || saved === 'undefined') return
if (saved == null) return null

try {
return JSON.parse(saved) as T
} catch (err) {
logError(Errors._700, `key ${key} – ${(err as Error).message}`)
}
return null
}

public setItem = <T>(key: string, item: T): void => {
const fullKey = this.getPrefixedKey(key)
try {
this.storage?.setItem(fullKey, JSON.stringify(item))
if (item == null) {
this.storage?.removeItem(fullKey)
} else {
this.storage?.setItem(fullKey, JSON.stringify(item))
}
} catch (err) {
logError(Errors._701, `key ${key} – ${(err as Error).message}`)
}
Expand Down
10 changes: 5 additions & 5 deletions src/services/local-storage/__tests__/local.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe('local storage', () => {

expect(getItem('test')).toStrictEqual({ test: 'value' })
})
it("returns undefined the key doesn't exist", () => {
expect(getItem('notAKey')).toBe(undefined)
it("returns null of the key doesn't exist", () => {
expect(getItem('notAKey')).toBe(null)
})
})

Expand All @@ -32,10 +32,10 @@ describe('local storage', () => {
})

describe('handling undefined', () => {
it('saves ands reads undefined', () => {
it('removes the key when passing undefined', () => {
setItem('test_undefined', undefined)
expect(getItem('test_undefined')).toBe(undefined)
expect(window.localStorage.getItem('SAFE_v2__test_undefined')).toBe('undefined')
expect(getItem('test_undefined')).toBe(null)
expect(window.localStorage.getItem('SAFE_v2__test_undefined')).toBe(null)
})
})
})
55 changes: 55 additions & 0 deletions src/services/local-storage/__tests__/useLocalStorage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { renderHook, act } from '@/tests/test-utils'
import local from '../local'
import useLocalStorage from '../useLocalStorage'

describe('useLocalStorage', () => {
beforeEach(() => {
window.localStorage.clear()
})

it('should set the value', () => {
const { result } = renderHook(() => useLocalStorage('test-key'))
const [value, setValue] = result.current

expect(value).toBe(undefined)

act(() => {
setValue('test')
})

expect(result.current[0]).toBe('test')

act(() => {
setValue('test2')
})

expect(result.current[0]).toBe('test2')
})

it('should set the value using a callback', () => {
const { result } = renderHook(() => useLocalStorage<string>('test-key'))
const [value, setValue] = result.current

expect(value).toBe(undefined)

act(() => {
setValue('test2')
})

act(() => {
setValue((prevVal) => {
return prevVal === 'test2' ? 'test3' : 'wrong'
})
})

expect(result.current[0]).toBe('test3')
})

it('should read from LS on initial call', () => {
local.setItem('test-key', 'ls')

const { result } = renderHook(() => useLocalStorage('test-key'))

expect(result.current[0]).toBe('ls')
})
})
70 changes: 52 additions & 18 deletions src/services/local-storage/useLocalStorage.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,66 @@
import type { Dispatch, SetStateAction } from 'react'
import { useState, useCallback, useEffect } from 'react'
import { useCallback, useEffect, useState } from 'react'
import local from './local'

const useLocalStorage = <T>(key: string, initialState: T): [T, Dispatch<SetStateAction<T>>] => {
const [cache, setCache] = useState<T>(initialState)
// The setter accepts T or a function that takes the old value and returns T
// Mimics the behavior of useState
type Undefinable<T> = T | undefined

useEffect(() => {
const initialValue = local.getItem<T>(key)
if (initialValue !== undefined) {
setCache(initialValue)
}
}, [setCache, key])
type Setter<T> = (val: T | ((prevVal: Undefinable<T>) => Undefinable<T>)) => void

const useLocalStorage = <T>(key: string): [Undefinable<T>, Setter<T>] => {
const [cache, setCache] = useState<Undefinable<T>>()

// This is the setter that will be returned
// It will update the local storage and cache
const setNewValue = useCallback<Setter<T>>(
(value) => {
setCache((oldValue) => {
const newValue = value instanceof Function ? value(oldValue) : value

const setNewValue = useCallback(
(value: T | ((prevState: T) => T)) => {
setCache((prevState) => {
const newState = value instanceof Function ? value(prevState) : value
if (newValue !== oldValue) {
local.setItem(key, newValue)

if (newState !== prevState) {
local.setItem(key, newState)
// Dispatch a fake storage event within the current browser tab
// The real storage event is dispatched only in other tabs
window.dispatchEvent(
new StorageEvent('storage', {
key: local.getPrefixedKey(key),
}),
)
}

return newState
return newValue
})
},
[setCache, key],
[key],
)

// Subscribe to changes in local storage and update the cache
// This will work across tabs
useEffect(() => {
const syncCache = () => {
const lsValue = local.getItem<T>(key)
if (lsValue !== null) {
setCache(lsValue)
}
}

const onStorageEvent = (event: StorageEvent) => {
if (event.key === local.getPrefixedKey(key)) {
syncCache()
}
}

// Set the initial value
syncCache()
usame-algan marked this conversation as resolved.
Show resolved Hide resolved

window.addEventListener('storage', onStorageEvent)

return () => {
window.removeEventListener('storage', onStorageEvent)
}
}, [key])

return [cache, setNewValue]
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/ls-migration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { MIGRATION_KEY } from './config'

const useStorageMigration = (): void => {
const dispatch = useAppDispatch()
const [isMigrationFinished, setIsMigrationFinished] = useLocalStorage<boolean | undefined>(MIGRATION_KEY, false)
const [isMigrationFinished = false, setIsMigrationFinished] = useLocalStorage<boolean>(MIGRATION_KEY)

useEffect(() => {
if (isMigrationFinished) return
Expand Down
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2463,13 +2463,20 @@
resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-modules-deployments/-/safe-modules-deployments-1.0.0.tgz#b4cfcf7782c3fa1491b84405ac06cfa4f7eff365"
integrity sha512-mZkGtfGmEym7l5w2K6d/mIaS8sDmy50uGWAfTj5Q/+Y1cU9fHYU6hzhb0QaeyNM7F7HM3jg5ZTJjartZJUqu3w==

"@gnosis.pm/safe-react-gateway-sdk@^3.1.3", "@gnosis.pm/safe-react-gateway-sdk@^3.4.1":
"@gnosis.pm/safe-react-gateway-sdk@^3.1.3":
version "3.4.1"
resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-react-gateway-sdk/-/safe-react-gateway-sdk-3.4.1.tgz#6864abdb28ce0487d64929dee0205e03a49e864f"
integrity sha512-b8i0BLEP9CC7S2N7hYDX3OXRYIb9asOMVW0QWh4jzsgT5xitUOPy4jrZL9kAmx8QgcMPYbD+Jjsq/R9ltzSmew==
dependencies:
cross-fetch "^3.1.5"

"@gnosis.pm/safe-react-gateway-sdk@^3.4.3":
version "3.4.3"
resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-react-gateway-sdk/-/safe-react-gateway-sdk-3.4.3.tgz#4551f75ee39a690f106804a5e19c9c59e2ba1ea1"
integrity sha512-KCyM+tl0Rw+Mv+N/yqNzCX3qm80WYhqPpJ3/vcV3jfxqvbBWsXN3z0TMOrx2oJb9621su8NiQmUaQAnLzv9SaQ==
dependencies:
cross-fetch "^3.1.5"

"@hapi/hoek@^9.0.0":
version "9.3.0"
resolved "https://registry.yarnpkg.com/@hapi/hoek/-/hoek-9.3.0.tgz#8368869dcb735be2e7f5cb7647de78e167a251fb"
Expand Down