diff --git a/src/useLocalStorageValue/index.ts b/src/useLocalStorageValue/index.ts index 0542a199..9d6bf6af 100644 --- a/src/useLocalStorageValue/index.ts +++ b/src/useLocalStorageValue/index.ts @@ -1,4 +1,8 @@ -import type {UseStorageValueOptions, UseStorageValueResult} from '../useStorageValue/index.js'; +import type { + UseStorageValueDeferredOptions, + UseStorageValueOptions, + UseStorageValueResult, +} from '../useStorageValue/index.js'; import {useStorageValue} from '../useStorageValue/index.js'; import {isBrowser, noop} from '../util/const.js'; @@ -10,20 +14,30 @@ try { IS_LOCAL_STORAGE_AVAILABLE = false; } -type UseLocalStorageValue = < - Type, - Default extends Type = Type, - Initialize extends boolean | undefined = boolean | undefined, ->( - key: string, - options?: UseStorageValueOptions, -) => UseStorageValueResult; +type UseLocalStorageValue = { + ( + key: string, + options: UseStorageValueDeferredOptions, + ): UseStorageValueResult; + ( + key: string, + options?: UseStorageValueOptions, + ): UseStorageValueResult; + ( + key: string, + options?: UseStorageValueOptions, + ): UseStorageValueResult; +}; /** * Manages a single localStorage key. */ export const useLocalStorageValue: UseLocalStorageValue = IS_LOCAL_STORAGE_AVAILABLE - ? (key, options) => useStorageValue(localStorage, key, options) + ? ( + key: string, + options?: UseStorageValueOptions, + ): UseStorageValueResult => + useStorageValue(localStorage, key, options) : ( _key: string, _options?: UseStorageValueOptions, diff --git a/src/useLocalStorageValue/index.types.test.ts b/src/useLocalStorageValue/index.types.test.ts new file mode 100644 index 00000000..7435b6e4 --- /dev/null +++ b/src/useLocalStorageValue/index.types.test.ts @@ -0,0 +1,30 @@ +import {expectTypeOf} from 'vitest'; +import {useLocalStorageValue} from './index.js'; + +declare const dynamicFlag: boolean; + +/** + * Type-level regression suite -- see `useStorageValue/index.types.test.ts` for + * the rationale. The wrapper declares its own overloads, so it needs its own + * assertions: a change to `useStorageValue` alone cannot keep it honest. + */ +export function useLocalStorageValueTypes(): void { + expectTypeOf(useLocalStorageValue('key').value).toEqualTypeOf(); + expectTypeOf(useLocalStorageValue('key', {defaultValue: 'default'}).value).toEqualTypeOf(); + expectTypeOf(useLocalStorageValue('key', {defaultValue: 'default'}).value).toEqualTypeOf(); + expectTypeOf(useLocalStorageValue('key', {initializeWithValue: true}).value).toEqualTypeOf(); + expectTypeOf(useLocalStorageValue('key', {initializeWithValue: undefined}).value).toEqualTypeOf(); + + expectTypeOf(useLocalStorageValue('key', {initializeWithValue: false}).value).toEqualTypeOf< + string | undefined + >(); + expectTypeOf(useLocalStorageValue('key', {defaultValue: 'default', initializeWithValue: false}).value).toEqualTypeOf< + string | undefined + >(); + expectTypeOf(useLocalStorageValue('key', {initializeWithValue: dynamicFlag}).value).toEqualTypeOf< + string | undefined + >(); + + expectTypeOf(useLocalStorageValue('key').value).toEqualTypeOf(); + expectTypeOf(useLocalStorageValue('key').value).toEqualTypeOf(); +} diff --git a/src/useSessionStorageValue/index.ts b/src/useSessionStorageValue/index.ts index 173041cb..2c2e54e8 100644 --- a/src/useSessionStorageValue/index.ts +++ b/src/useSessionStorageValue/index.ts @@ -1,4 +1,8 @@ -import type {UseStorageValueOptions, UseStorageValueResult} from '../useStorageValue/index.js'; +import type { + UseStorageValueDeferredOptions, + UseStorageValueOptions, + UseStorageValueResult, +} from '../useStorageValue/index.js'; import {useStorageValue} from '../useStorageValue/index.js'; import {isBrowser, noop} from '../util/const.js'; @@ -10,20 +14,30 @@ try { IS_SESSION_STORAGE_AVAILABLE = false; } -type UseSessionStorageValue = < - Type, - Default extends Type = Type, - Initialize extends boolean | undefined = boolean | undefined, ->( - key: string, - options?: UseStorageValueOptions, -) => UseStorageValueResult; +type UseSessionStorageValue = { + ( + key: string, + options: UseStorageValueDeferredOptions, + ): UseStorageValueResult; + ( + key: string, + options?: UseStorageValueOptions, + ): UseStorageValueResult; + ( + key: string, + options?: UseStorageValueOptions, + ): UseStorageValueResult; +}; /** * Manages a single sessionStorage key. */ export const useSessionStorageValue: UseSessionStorageValue = IS_SESSION_STORAGE_AVAILABLE - ? (key, options) => useStorageValue(sessionStorage, key, options) + ? ( + key: string, + options?: UseStorageValueOptions, + ): UseStorageValueResult => + useStorageValue(sessionStorage, key, options) : ( _key: string, _options?: UseStorageValueOptions, diff --git a/src/useSessionStorageValue/index.types.test.ts b/src/useSessionStorageValue/index.types.test.ts new file mode 100644 index 00000000..d66f8839 --- /dev/null +++ b/src/useSessionStorageValue/index.types.test.ts @@ -0,0 +1,30 @@ +import {expectTypeOf} from 'vitest'; +import {useSessionStorageValue} from './index.js'; + +declare const dynamicFlag: boolean; + +/** + * Type-level regression suite -- see `useStorageValue/index.types.test.ts` for + * the rationale. The wrapper declares its own overloads, so it needs its own + * assertions: a change to `useStorageValue` alone cannot keep it honest. + */ +export function useSessionStorageValueTypes(): void { + expectTypeOf(useSessionStorageValue('key').value).toEqualTypeOf(); + expectTypeOf(useSessionStorageValue('key', {defaultValue: 'default'}).value).toEqualTypeOf(); + expectTypeOf(useSessionStorageValue('key', {defaultValue: 'default'}).value).toEqualTypeOf(); + expectTypeOf(useSessionStorageValue('key', {initializeWithValue: true}).value).toEqualTypeOf(); + expectTypeOf(useSessionStorageValue('key', {initializeWithValue: undefined}).value).toEqualTypeOf(); + + expectTypeOf(useSessionStorageValue('key', {initializeWithValue: false}).value).toEqualTypeOf< + string | undefined + >(); + expectTypeOf( + useSessionStorageValue('key', {defaultValue: 'default', initializeWithValue: false}).value, + ).toEqualTypeOf(); + expectTypeOf(useSessionStorageValue('key', {initializeWithValue: dynamicFlag}).value).toEqualTypeOf< + string | undefined + >(); + + expectTypeOf(useSessionStorageValue('key').value).toEqualTypeOf(); + expectTypeOf(useSessionStorageValue('key').value).toEqualTypeOf(); +} diff --git a/src/useStorageValue/index.dom.test.ts b/src/useStorageValue/index.dom.test.ts index 03546f02..bcd74164 100644 --- a/src/useStorageValue/index.dom.test.ts +++ b/src/useStorageValue/index.dom.test.ts @@ -108,6 +108,29 @@ describe('useStorageValue', () => { expect(expectResultValue(result.all[0]).value).toBe('bar'); }); + it('should fetch value on first render in case `initializeWithValue` option is set to undefined', async () => { + const {result} = await renderHook(() => + useStorageValue( + newStorage(() => '"bar"'), + 'foo', + {initializeWithValue: undefined}, + ), + ); + + expect(expectResultValue(result.all[0]).value).toBe('bar'); + }); + + it('should yield null in case `defaultValue` option is set to undefined', async () => { + const {result} = await renderHook(() => + // `exactOptionalPropertyTypes` rejects the explicit `undefined` here, but + // consumers without it -- and every JS consumer -- can still pass one. + // @ts-expect-error -- deliberately passing what the option type forbids + useStorageValue(newStorage(), 'foo', {defaultValue: undefined}), + ); + + expect(expectResultValue(result).value).toBe(null); + }); + it('should set storage value on .set() call', async () => { const {result} = await renderHook(() => useStorageValue(newStorage(), 'foo')); diff --git a/src/useStorageValue/index.ts b/src/useStorageValue/index.ts index 65d7fad6..c0c1b464 100644 --- a/src/useStorageValue/index.ts +++ b/src/useStorageValue/index.ts @@ -109,6 +109,17 @@ export type UseStorageValueOptions string | null; }; +/** + * Options of a hook that defers the first storage read until effects run. + * + * `initializeWithValue` is required so that an options object that omits it + * cannot match the deferred overload -- matching it would put `undefined` back + * into the result type of a hook that does read the value on the first render. + */ +export type UseStorageValueDeferredOptions = UseStorageValueOptions & { + initializeWithValue: false; +}; + type UseStorageValueValue< Type, Default extends Type = Type, @@ -129,11 +140,29 @@ export type UseStorageValueResult< fetch: () => void; }; -const DEFAULT_OPTIONS = { - defaultValue: null, - initializeWithValue: true, -}; +export function useStorageValue( + storage: Storage, + key: string, + options: UseStorageValueDeferredOptions, +): UseStorageValueResult; +export function useStorageValue( + storage: Storage, + key: string, + options?: UseStorageValueOptions, +): UseStorageValueResult; +export function useStorageValue< + Type, + Default extends Type = Type, + Initialize extends boolean | undefined = boolean | undefined, +>( + storage: Storage, + key: string, + options?: UseStorageValueOptions, +): UseStorageValueResult; +/** + * Manages a single storage key. + */ export function useStorageValue< Type, Default extends Type = Type, @@ -143,7 +172,13 @@ export function useStorageValue< key: string, options?: UseStorageValueOptions, ): UseStorageValueResult { - const optionsRef = useSyncedRef({...DEFAULT_OPTIONS, ...options}); + const optionsRef = useSyncedRef({ + ...options, + // An explicitly passed `undefined` must fall back to the documented + // default instead of overriding it, the way a spread over defaults would. + defaultValue: options?.defaultValue ?? null, + initializeWithValue: options?.initializeWithValue ?? true, + }); const parse = (str: string | null, fallback: Type | null): Type | null => { const parseFunction = optionsRef.current.parse ?? defaultParse; return parseFunction(str, fallback); diff --git a/src/useStorageValue/index.types.test.ts b/src/useStorageValue/index.types.test.ts new file mode 100644 index 00000000..becdb015 --- /dev/null +++ b/src/useStorageValue/index.types.test.ts @@ -0,0 +1,80 @@ +import {expectTypeOf} from 'vitest'; +import type {NextState} from '../util/resolve-hook-state.js'; +import type {UseStorageValueResult} from './index.js'; +import {useStorageValue} from './index.js'; + +declare const storage: Storage; +declare const dynamicFlag: boolean; + +/** + * Type-level regression suite for the `value` field of the hook result. + * + * Nothing here runs -- the assertions are checked by the type-check pass of + * `vp lint`. `undefined` may only appear in `value` when `initializeWithValue` + * is not known to resolve to `true`, since deferring the first read until + * effects run is the only case that yields `undefined` to the caller. + * + * TypeScript does not infer a trailing type parameter when an earlier one is + * passed explicitly, so every case below is repeated with an explicit `Type` -- + * that is the shape in which the bug originally surfaced. + */ +export function useStorageValueValueTypes(): void { + // `initializeWithValue` omitted -- the value is read during the first render. + expectTypeOf(useStorageValue(storage, 'key').value).toEqualTypeOf(); + expectTypeOf(useStorageValue(storage, 'key', {defaultValue: 'default'}).value).toEqualTypeOf(); + expectTypeOf(useStorageValue(storage, 'key', {defaultValue: 'default'}).value).toEqualTypeOf(); + + // Explicit `true` behaves like the omitted case. + expectTypeOf( + useStorageValue(storage, 'key', {defaultValue: 'default', initializeWithValue: true}).value, + ).toEqualTypeOf(); + expectTypeOf(useStorageValue(storage, 'key', {initializeWithValue: true}).value).toEqualTypeOf(); + + // An explicit `undefined` falls back to the default, so the value is read too. + expectTypeOf(useStorageValue(storage, 'key', {initializeWithValue: undefined}).value).toEqualTypeOf(); + + // Explicit `false` defers the read, so the first render yields `undefined`. + expectTypeOf(useStorageValue(storage, 'key', {defaultValue: 'default', initializeWithValue: false}).value) + // + .toEqualTypeOf(); + expectTypeOf(useStorageValue(storage, 'key', {initializeWithValue: false}).value).toEqualTypeOf< + string | undefined + >(); + + // A flag unknown at compile time may turn out to be `false`. + expectTypeOf(useStorageValue(storage, 'key', {defaultValue: 'default', initializeWithValue: dynamicFlag}).value) + // + .toEqualTypeOf(); + expectTypeOf(useStorageValue(storage, 'key', {initializeWithValue: dynamicFlag}).value).toEqualTypeOf< + string | undefined + >(); + + // Explicit type arguments keep addressing the same overload they used to. + expectTypeOf(useStorageValue(storage, 'key').value).toEqualTypeOf(); + expectTypeOf(useStorageValue(storage, 'key').value).toEqualTypeOf(); +} + +/** + * `set` receives the value type the hook yields, so an updater callback must not + * be handed an `undefined` previous state unless the read is deferred. + */ +export function useStorageValueSetTypes(): void { + expectTypeOf(useStorageValue(storage, 'key').set).parameter(0).toEqualTypeOf>(); + + expectTypeOf(useStorageValue(storage, 'key', {initializeWithValue: false}).set) + .parameter(0) + .toEqualTypeOf>(); +} + +/** + * The exported result type keeps its three parameters, and instantiating it by + * hand -- the way a consumer annotates a variable -- resolves the same way. + */ +export function useStorageValueResultTypes(): void { + expectTypeOf['value']>().toEqualTypeOf(); + expectTypeOf['value']>().toEqualTypeOf(); + expectTypeOf['value']>().toEqualTypeOf(); + + // Without a known `initializeWithValue`, the type stays conservative. + expectTypeOf['value']>().toEqualTypeOf(); +}