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

On error 2 #76

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
42 changes: 38 additions & 4 deletions src/useLocalStorageState.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import type { Dispatch, SetStateAction } from 'react'
import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react'

Expand All @@ -12,6 +13,7 @@ export type LocalStorageOptions<T> = {
stringify: (value: unknown) => string
parse: (value: string) => unknown
}
onError?: (err: unknown, oldValue: T | undefined, newValue: string) => T | undefined
}

// - `useLocalStorageState()` return type
Expand Down Expand Up @@ -44,10 +46,17 @@ export default function useLocalStorageState<T = undefined>(
const serializer = options?.serializer
const [defaultValue] = useState(options?.defaultValue)
const [defaultServerValue] = useState(options?.defaultServerValue)

const defaultOnError = (err: unknown): T | undefined => {
// eslint-disable-next-line no-console
throw err
}

return useLocalStorage(
key,
defaultValue,
defaultServerValue,
options?.onError ?? defaultOnError,
options?.storageSync,
serializer?.parse,
serializer?.stringify,
Expand All @@ -58,6 +67,7 @@ function useLocalStorage<T>(
key: string,
defaultValue: T | undefined,
defaultServerValue: T | undefined,
onError: Required<LocalStorageOptions<T>>['onError'],
storageSync: boolean = true,
parse: (value: string) => unknown = parseJSON,
stringify: (value: unknown) => string = JSON.stringify,
Expand All @@ -68,6 +78,13 @@ function useLocalStorage<T>(
parsed: undefined,
})

const [error, setError] = useState<null | Error>(null)

// Nice little React short cut - we can ignore rules of hooks if we are throwing errors.
if(error){
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this makes sense to me, but I'm wondering if it's a bit unconventional.... do people expect hooks to throw errors?

throw error;
}

const value = useSyncExternalStore(
// useSyncExternalStore.subscribe
useCallback(
Expand All @@ -94,12 +111,28 @@ function useLocalStorage<T>(
} else if (string !== storageItem.current.string) {
let parsed: T | undefined

try {
parsed = string === null ? defaultValue : (parse(string) as T)
} catch {
if (string === null) {
parsed = defaultValue
} else {
try {
parsed = parse(string) as T
} catch (err) {

// Nested try catch seems crazy, but the idea is, if the `onError` itself throws an error,
// Then we treat it like an error, and set it into seperate state.
// This is more intuitive from the users point of view, where if they throw from their onError function,
// Then they get throw like functionality.
try {
parsed = onError(err, storageItem.current.parsed, string)
} catch (err_ ) {
if (err_ instanceof Error) {
setError(err_)
} else {
setError(new Error(`Unknown error occured during parsing., value was ${err_}`))
}
}
}
}

storageItem.current.parsed = parsed
}

Expand Down Expand Up @@ -171,6 +204,7 @@ function useLocalStorage<T>(
}

const onStorage = (e: StorageEvent): void => {
console.log('storage')
if (e.key === key && e.storageArea === goodTry(() => localStorage)) {
triggerCallbacks(key)
}
Expand Down
102 changes: 98 additions & 4 deletions test/browser.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

import util from 'node:util'
import superjson from 'superjson'
import { act, render, renderHook } from '@testing-library/react'
import { act, render, renderHook, waitFor } from '@testing-library/react'
import React, { useEffect, useLayoutEffect, useMemo } from 'react'
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
import useLocalStorageState, { inMemoryData } from '../src/useLocalStorageState.js'

let originalError = console.error;

beforeEach(() => {
// Throw an error when `console.error()` is called. This is especially useful in a React tests
// because React uses it to show warnings and discourage you from shooting yourself in the foot.
Expand All @@ -25,7 +27,13 @@ beforeEach(() => {
// (`Component`). To locate the bad setState() call inside `Component`, follow the stack trace
// as described in https://reactjs.org/link/setstate-in-render"
vi.spyOn(console, 'error').mockImplementation((format: string, ...args: any[]) => {
throw new Error(util.format(format, ...args))

// Just commenting this out, but basically add exceptions to this
//throw new Error(util.format(format, ...args))

originalError(format, ...args);


})
})

Expand All @@ -34,7 +42,7 @@ afterEach(() => {
try {
localStorage.clear()
sessionStorage.clear()
} catch {}
} catch { }
})

describe('useLocalStorageState()', () => {
Expand Down Expand Up @@ -758,8 +766,94 @@ describe('useLocalStorageState()', () => {
serializer: JSON,
}),
)
const [value] = resultB.current
const [value] = resultB.current;
expect(value).toEqual(['first', 'second'])
expect(value).not.toBe(undefined)
})
})

describe.only('"onError" option', () => {

// I really want to be able to write a test like this!
// But it's not working? JSDOM has implemented the storageEvent thought:
// https://github.com/jsdom/jsdom/pull/2076
test.skip("sanity test", async () => {
const renderResult = renderHook(() =>
useLocalStorageState<string>('todos', {
defaultValue: "hello world",
serializer: {
stringify: (v) => v as string,
parse: v => v
}

}));

expect(renderResult.result.current[0]).toBe("hello world");

localStorage.setItem("todos", "123");
await waitFor(() => {
return expect(renderResult.result.current[0]).toBe("123");

}); })


test("opt in for console.error", async () => {

localStorage.setItem("some-string", "xyz")
const { result: resultA, unmount } = renderHook(() =>
useLocalStorageState<string>('some-string', {
defaultValue: "abc",
onError: (err) => {
console.error(err);
return "abc";
}
}),
)

expect(resultA.current[0]).toBe("abc")

const calls = vi.mocked(console.error).mock.calls;
expect(calls).toHaveLength(1);
expect(calls[0][0]).toBeInstanceOf(Error);
expect((calls[0][0] as Error).message).toBe("Unexpected token 'x', \"xyz\" is not valid JSON");
})

test("default behaviour is parsing errors will cause a runtime error", async () => {

localStorage.setItem("some-string", "xyz")
expect(() => renderHook(() =>
useLocalStorageState<string>('some-string', {
defaultValue: "abc",
}),
)).toThrow();

})

test("custom onError logic can be provided", async () => {

localStorage.setItem("some-string", "xyz")

const mockFn = vi.fn().mockReturnValue("error fallback");
const { result: resultA, unmount } = renderHook(() =>
useLocalStorageState<string>('some-string', {
defaultValue: "abc",
onError: mockFn

}),
)

expect(resultA.current[0]).toBe("error fallback")

const calls = vi.mocked(mockFn).mock.calls;
expect(calls).toHaveLength(1);
expect(calls[0][0]).toBeInstanceOf(Error);
expect((calls[0][0] as Error).message).toBe("Unexpected token 'x', \"xyz\" is not valid JSON");
expect(calls[0][1]).toBe(undefined);
expect(calls[0][2]).toBe("xyz")

expect(console.error).not.toHaveBeenCalled();

})
});

})