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

fix: proper localstorage usage #5382

Closed
wants to merge 2 commits into from
Closed
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
17 changes: 6 additions & 11 deletions app/components/mask.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import { Updater } from "../typing";
import { ModelConfigList } from "./model-config";
import { FileName, Path } from "../constant";
import { BUILTIN_MASK_STORE } from "../masks";
import { nanoid } from "nanoid";
import { useLocalStorage } from "../utils";
import {
DragDropContext,
Droppable,
Expand Down Expand Up @@ -426,16 +426,11 @@ export function MaskPage() {
const maskStore = useMaskStore();
const chatStore = useChatStore();

const [filterLang, setFilterLang] = useState<Lang | undefined>(
() => localStorage.getItem("Mask-language") as Lang | undefined,
const [filterLang, setFilterLang] = useLocalStorage<Lang | null>(
"Mask-language",
null,
{ raw: true },
);
useEffect(() => {
if (filterLang) {
localStorage.setItem("Mask-language", filterLang);
} else {
localStorage.removeItem("Mask-language");
}
}, [filterLang]);

const allMasks = maskStore
.getAll()
Expand Down Expand Up @@ -542,7 +537,7 @@ export function MaskPage() {
onChange={(e) => {
const value = e.currentTarget.value;
if (value === Locale.Settings.Lang.All) {
setFilterLang(undefined);
setFilterLang(null);
} else {
setFilterLang(value as Lang);
}
Expand Down
6 changes: 6 additions & 0 deletions app/locales/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ merge(fallbackLang, targetLang);
export default fallbackLang as LocaleType;

function getItem(key: string) {
if (typeof window === "undefined") {
return null;
}
try {
return localStorage.getItem(key);
} catch {
Expand All @@ -90,6 +93,9 @@ function getItem(key: string) {
}

function setItem(key: string, value: string) {
if (typeof window === "undefined") {
return null;
}
try {
localStorage.setItem(key, value);
} catch {}
Expand Down
305 changes: 304 additions & 1 deletion app/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { useEffect, useState } from "react";
import {
useEffect,
useLayoutEffect,
useState,
useSyncExternalStore,
useCallback,
useMemo,
} from "react";
import { showToast } from "./components/ui-lib";
import Locale from "./locales";
import { RequestMessage } from "./client/api";
Expand Down Expand Up @@ -318,3 +325,299 @@ export function adapter(config: Record<string, unknown>) {
: path;
return fetch(fetchUrl as string, { ...rest, responseType: "text" });
}

/**
* Copyright 2024 Sukka (https://skk.moe) and the contributors of foxact (https://foxact.skk.moe)
* Licensed under the MIT License
*/

const useIsomorphicLayoutEffect =
typeof window !== "undefined" ? useLayoutEffect : useEffect;

const noop = () => {};

const stlProp = Object.getOwnPropertyDescriptor(Error, "stackTraceLimit");
const hasSTL = stlProp?.writable && typeof stlProp.value === "number";
const noSSRError = (
errorMessage?: string | undefined,
nextjsDigest = "BAILOUT_TO_CLIENT_SIDE_RENDERING",
) => {
const originalStackTraceLimit = Error.stackTraceLimit;

/**
* This is *only* safe to do when we know that nothing at any point in the
* stack relies on the `Error.stack` property of the noSSRError. By removing
* the strack trace of the error, we can improve the performance of object
* creation by a lot.
*/
if (hasSTL) {
Error.stackTraceLimit = 0;
}

const error = new Error(errorMessage);

/**
* Restore the stack trace limit to its original value after the error has
* been created.
*/
if (hasSTL) {
Error.stackTraceLimit = originalStackTraceLimit;
}

// Next.js marks errors with `NEXT_DYNAMIC_NO_SSR_CODE` digest as recoverable:
// https://github.com/vercel/next.js/blob/bef716ad031591bdf94058aaf4b8d842e75900b5/packages/next/src/shared/lib/lazy-dynamic/bailout-to-csr.ts#L2
(error as any).digest = nextjsDigest;

(error as any).recoverableError = "NO_SSR";

return error;
};

Comment on lines +334 to +374
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor Error Handling for SSR Compatibility

The noSSRError function is designed to handle errors specifically for server-side rendering scenarios by manipulating the Error.stackTraceLimit. This approach is clever as it reduces the performance overhead of error handling by limiting the stack trace. However, this might obscure useful debugging information in some cases.

  • Pros: Reduces overhead by limiting the stack trace.
  • Cons: Potentially loses valuable debugging information.

Consider providing a configuration option to toggle this behavior based on the environment or the severity of the error, allowing developers more control over error reporting.

type StorageType = "localStorage" | "sessionStorage";
type NotUndefined<T> = T extends undefined ? never : T;

// StorageEvent is deliberately not fired on the same document, we do not want to change that
type CustomStorageEvent = CustomEvent<string>;
declare global {
interface WindowEventMap {
"foxact-use-local-storage": CustomStorageEvent;
"foxact-use-session-storage": CustomStorageEvent;
}
}

export type Serializer<T> = (value: T) => string;
export type Deserializer<T> = (value: string) => T;

// This type utility is only used for workaround https://github.com/microsoft/TypeScript/issues/37663
const isFunction = (x: unknown): x is Function => typeof x === "function";

Copy link
Contributor

Choose a reason for hiding this comment

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

Type Definition Issue: Avoid Using 'Function' as a Type

The use of Function as a type for the isFunction utility (line 392) is flagged by static analysis tools. Using Function can lead to maintenance issues and bugs because it is too generic.

  • Pros: Quick and easy way to check for function types.
  • Cons: Lacks specificity, which can lead to bugs if non-function values that are callable (like classes) are passed.

Consider replacing Function with a more specific function type definition that includes the expected arguments and return type, enhancing type safety and clarity.

- const isFunction = (x: unknown): x is Function => typeof x === 'function';
+ const isFunction = (x: unknown): x is (...args: any[]) => any => typeof x === 'function';
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isFunction = (x: unknown): x is Function => typeof x === "function";
const isFunction = (x: unknown): x is (...args: any[]) => any => typeof x === "function";
Tools
Biome

[error] 392-392: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)

const identity = (x: any) => x;

export interface UseStorageRawOption {
raw: true;
}

export interface UseStorageParserOption<T> {
raw?: false;
serializer: Serializer<T>;
deserializer: Deserializer<T>;
}

const getServerSnapshotWithoutServerValue = () => {
throw noSSRError(
"useLocalStorage cannot be used on the server without a serverValue",
);
};

function createStorage(type: StorageType) {
const FOXACT_LOCAL_STORAGE_EVENT_KEY =
type === "localStorage"
? "foxact-use-local-storage"
: "foxact-use-session-storage";

const foxactHookName =
type === "localStorage"
? "foxact/use-local-storage"
: "foxact/use-session-storage";

const dispatchStorageEvent =
typeof window !== "undefined"
? (key: string) => {
window.dispatchEvent(
new CustomEvent<string>(FOXACT_LOCAL_STORAGE_EVENT_KEY, {
detail: key,
}),
);
}
: noop;

const setStorageItem =
typeof window !== "undefined"
? (key: string, value: string) => {
try {
window[type].setItem(key, value);
} catch {
console.warn(
`[${foxactHookName}] Failed to set value to ${type}, it might be blocked`,
);
} finally {
dispatchStorageEvent(key);
}
}
: noop;

const removeStorageItem =
typeof window !== "undefined"
? (key: string) => {
try {
window[type].removeItem(key);
} catch {
console.warn(
`[${foxactHookName}] Failed to remove value from ${type}, it might be blocked`,
);
} finally {
dispatchStorageEvent(key);
}
}
: noop;

const getStorageItem = (key: string) => {
if (typeof window === "undefined") {
return null;
}
try {
return window[type].getItem(key);
} catch {
console.warn(
`[${foxactHookName}] Failed to get value from ${type}, it might be blocked`,
);
return null;
}
};

const useSetStorage = <T>(key: string, serializer: Serializer<T>) =>
useCallback(
(v: T | null) => {
try {
if (v === null) {
removeStorageItem(key);
} else {
setStorageItem(key, serializer(v));
}
} catch (e) {
console.warn(e);
}
},
[key, serializer],
);

// ssr compatible
function useStorage<T>(
key: string,
serverValue: NotUndefined<T>,
options?: UseStorageRawOption | UseStorageParserOption<T>,
): readonly [T, React.Dispatch<React.SetStateAction<T | null>>];
// client-render only
function useStorage<T>(
key: string,
serverValue?: undefined,
options?: UseStorageRawOption | UseStorageParserOption<T>,
): readonly [T | null, React.Dispatch<React.SetStateAction<T | null>>];
function useStorage<T>(
key: string,
serverValue?: NotUndefined<T> | undefined,
options: UseStorageRawOption | UseStorageParserOption<T> = {
serializer: JSON.stringify,
deserializer: JSON.parse,
},
):
| readonly [T | null, React.Dispatch<React.SetStateAction<T | null>>]
| readonly [T, React.Dispatch<React.SetStateAction<T | null>>] {
const subscribeToSpecificKeyOfLocalStorage = useCallback(
(callback: () => void) => {
if (typeof window === "undefined") {
return noop;
}

const handleStorageEvent = (e: StorageEvent) => {
if (
!("key" in e) || // Some browsers' strange quirk where StorageEvent is missing key
e.key === key
) {
callback();
}
};
const handleCustomStorageEvent = (e: CustomStorageEvent) => {
if (e.detail === key) {
callback();
}
};

window.addEventListener("storage", handleStorageEvent);
window.addEventListener(
FOXACT_LOCAL_STORAGE_EVENT_KEY,
handleCustomStorageEvent,
);
return () => {
window.removeEventListener("storage", handleStorageEvent);
window.removeEventListener(
FOXACT_LOCAL_STORAGE_EVENT_KEY,
handleCustomStorageEvent,
);
};
},
[key],
);

const serializer: Serializer<T> = options.raw
? identity
: options.serializer;
const deserializer: Deserializer<T> = options.raw
? identity
: options.deserializer;

const getClientSnapshot = () => getStorageItem(key);

// If the serverValue is provided, we pass it to useSES' getServerSnapshot, which will be used during SSR
// If the serverValue is not provided, we don't pass it to useSES, which will cause useSES to opt-in client-side rendering
const getServerSnapshot =
serverValue !== undefined
? () => serializer(serverValue)
: getServerSnapshotWithoutServerValue;

const store = useSyncExternalStore(
subscribeToSpecificKeyOfLocalStorage,
getClientSnapshot,
getServerSnapshot,
);

const deserialized = useMemo(
() => (store === null ? null : deserializer(store)),
[store, deserializer],
);

const setState = useCallback<
React.Dispatch<React.SetStateAction<T | null>>
>(
(v) => {
try {
const nextState = isFunction(v) ? v(deserialized ?? null) : v;

if (nextState === null) {
removeStorageItem(key);
} else {
setStorageItem(key, serializer(nextState));
}
} catch (e) {
console.warn(e);
}
},
[key, serializer, deserialized],
);

useLayoutEffect(() => {
if (getStorageItem(key) === null && serverValue !== undefined) {
setStorageItem(key, serializer(serverValue));
}
}, [deserializer, key, serializer, serverValue]);

const finalValue: T | null =
deserialized === null
? // storage doesn't have value
serverValue === undefined
? // no default value provided
null
: (serverValue satisfies NotUndefined<T>)
: // storage has value
(deserialized satisfies T);

return [finalValue, setState] as const;
}

return {
useStorage,
useSetStorage,
};
}

export const useLocalStorage = createStorage("localStorage").useStorage;
Loading
Loading