From 2f3d89c84c70eaf99ca7c041dfb16131b5e77d6f Mon Sep 17 00:00:00 2001 From: aster <137767097+aster-void@users.noreply.github.com> Date: Fri, 6 Jun 2025 11:20:03 +0900 Subject: [PATCH 1/4] test: PersistedState should work when values are null or undefined --- .../persisted-state.test.svelte.ts | 113 +++++++++++------- 1 file changed, 67 insertions(+), 46 deletions(-) diff --git a/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts b/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts index 64feb88b..7eef104f 100644 --- a/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts +++ b/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts @@ -28,6 +28,15 @@ describe("PersistedState", async () => { expect(persistedState.current).toBe(initialValue); }); + testWithEffect("setting null or undefined should update it", () => { + const persistedState = new PersistedState(key, initialValue); + const derivedVal = $derived(persistedState.current); // this makes sure that signals are fired + persistedState.current = null; + expect(derivedVal).toBe(null); + persistedState.current = undefined; + expect(derivedVal).toBe(undefined); + }); + describe("localStorage", () => { testWithEffect("uses initial value if no persisted value is found", () => { const persistedState = new PersistedState(key, initialValue); @@ -49,18 +58,24 @@ describe("PersistedState", async () => { expect(localStorage.getItem(key)).toBe(JSON.stringify(newValue)); }); - testWithEffect("updates localStorage when a nested property in current value changes", () => { - const propValue = "test"; - const initialValue = { prop: { nested: propValue } }; - const newPropValue = "new test"; - const newValue = { prop: { nested: newPropValue } }; - const persistedState = new PersistedState(key, initialValue); - expect(persistedState.current).toEqual(initialValue); + testWithEffect( + "updates localStorage when a nested property in current value changes", + () => { + const propValue = "test"; + const initialValue = { prop: { nested: propValue } }; + const newPropValue = "new test"; + const newValue = { prop: { nested: newPropValue } }; + const persistedState = new PersistedState<{ prop: { nested: string } }>( + key, + initialValue, + ); + expect(persistedState.current).toEqual(initialValue); - persistedState.current.prop.nested = newPropValue; - expect(persistedState.current).toEqual(newValue); - expect(localStorage.getItem(key)).toBe(JSON.stringify(newValue)); - }); + persistedState.current.prop.nested = newPropValue; + expect(persistedState.current).toEqual(newValue); + expect(localStorage.getItem(key)).toBe(JSON.stringify(newValue)); + }, + ); testWithEffect("updates current value when localStorage changes", () => { const propValue = "test"; @@ -125,22 +140,25 @@ describe("PersistedState", async () => { }); describe("syncTabs", () => { - testWithEffect("updates persisted value when local storage changes independently", () => { - $effect(() => { - const persistedState = new PersistedState(key, initialValue); - expect(persistedState.current).toBe(initialValue); - - localStorage.setItem(key, JSON.stringify(newValue)); - window.dispatchEvent( - new StorageEvent("storage", { - key, - oldValue: null, - newValue: JSON.stringify(newValue), - }) - ); - expect(persistedState.current).toBe(newValue); - }); - }); + testWithEffect( + "updates persisted value when local storage changes independently", + () => { + $effect(() => { + const persistedState = new PersistedState(key, initialValue); + expect(persistedState.current).toBe(initialValue); + + localStorage.setItem(key, JSON.stringify(newValue)); + window.dispatchEvent( + new StorageEvent("storage", { + key, + oldValue: null, + newValue: JSON.stringify(newValue), + }), + ); + expect(persistedState.current).toBe(newValue); + }); + }, + ); testWithEffect( "does not update persisted value when local storage changes independently if syncTabs is false", @@ -157,30 +175,33 @@ describe("PersistedState", async () => { key, oldValue: null, newValue: JSON.stringify(newValue), - }) + }), ); expect(persistedState.current).toBe(initialValue); }); - } + }, ); - testWithEffect("does not handle the storage event when 'session' storage is used", () => { - $effect(() => { - const persistedState = new PersistedState(key, initialValue, { - storage: "session", + testWithEffect( + "does not handle the storage event when 'session' storage is used", + () => { + $effect(() => { + const persistedState = new PersistedState(key, initialValue, { + storage: "session", + }); + expect(persistedState.current).toBe(initialValue); + + sessionStorage.setItem(key, JSON.stringify(newValue)); + window.dispatchEvent( + new StorageEvent("storage", { + key, + oldValue: null, + newValue: JSON.stringify(newValue), + }), + ); + expect(persistedState.current).toBe(initialValue); }); - expect(persistedState.current).toBe(initialValue); - - sessionStorage.setItem(key, JSON.stringify(newValue)); - window.dispatchEvent( - new StorageEvent("storage", { - key, - oldValue: null, - newValue: JSON.stringify(newValue), - }) - ); - expect(persistedState.current).toBe(initialValue); - }); - }); + }, + ); }); }); From 358d7c1619df772c1366053231a142c0b4a446b6 Mon Sep 17 00:00:00 2001 From: aster <137767097+aster-void@users.noreply.github.com> Date: Fri, 6 Jun 2025 11:25:35 +0900 Subject: [PATCH 2/4] breaking: PersistedState now handles null and undefined correctly --- .../persisted-state/persisted-state.svelte.ts | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts b/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts index 5ba3f0e3..a10f2395 100644 --- a/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts +++ b/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts @@ -4,11 +4,15 @@ import { createSubscriber } from "svelte/reactivity"; type Serializer = { serialize: (value: T) => string; - deserialize: (value: string) => T | undefined; + deserialize: (value: string) => T; }; type StorageType = "local" | "session"; +// use this on error instead of `undefined` to avoid false positives in the deserialization error check +const DESERIALIZATION_ERROR = Symbol("[persisted-state] deserialization error"); +type DESERIALIZATION_ERROR = typeof DESERIALIZATION_ERROR; + function getStorage(storageType: StorageType, window: Window & typeof globalThis): Storage { switch (storageType) { case "local": @@ -21,7 +25,7 @@ function getStorage(storageType: StorageType, window: Window & typeof globalThis type PersistedStateOptions = { /** The storage type to use. Defaults to `local`. */ storage?: StorageType; - /** The serializer to use. Defaults to `JSON.stringify` and `JSON.parse`. */ + /** The serializer to use. Defaults to `JSON.stringify` and `JSON.parse` with slight modification. */ serializer?: Serializer; /** Whether to sync with the state changes from other tabs. Defaults to `true`. */ syncTabs?: boolean; @@ -36,21 +40,26 @@ type PersistedStateOptions = { * @see {@link https://runed.dev/docs/utilities/persisted-state} */ export class PersistedState { - #current: T | undefined; + #current: T; #key: string; #serializer: Serializer; #storage?: Storage; #subscribe?: VoidFunction; + #initialValue: T; #version = $state(0); constructor(key: string, initialValue: T, options: PersistedStateOptions = {}) { const { storage: storageType = "local", - serializer = { serialize: JSON.stringify, deserialize: JSON.parse }, + serializer = { serialize: JSON.stringify, deserialize: (val) => { + if (val === "undefined") return undefined; // JSON.parse can't parse "undefined", but JSON.stringify will serialize undefined to "undefined" + return JSON.parse(val); + } }, syncTabs = true, } = options; const window = "window" in options ? options.window : defaultWindow; // window is not mockable to be undefined without this, because JavaScript will fill undefined with `= default` + this.#initialValue = initialValue; this.#current = initialValue; this.#key = key; this.#serializer = serializer; @@ -62,7 +71,10 @@ export class PersistedState { const existingValue = storage.getItem(key); if (existingValue !== null) { - this.#current = this.#deserialize(existingValue); + const deserialized = this.#deserialize(existingValue); + if (deserialized !== DESERIALIZATION_ERROR) { + this.#current = deserialized; + } } else { this.#serialize(initialValue); } @@ -79,7 +91,10 @@ export class PersistedState { this.#version; const storageItem = this.#storage?.getItem(this.#key); - const root = storageItem ? this.#deserialize(storageItem) : this.#current; + let root = storageItem ? this.#deserialize(storageItem) : this.#current; + if (root === DESERIALIZATION_ERROR) { + root = this.#initialValue; + } const proxies = new WeakMap(); const proxy = (value: unknown) => { @@ -113,25 +128,27 @@ export class PersistedState { } #handleStorageEvent = (event: StorageEvent): void => { - if (event.key !== this.#key || event.newValue === null) return; - this.#current = this.#deserialize(event.newValue); - this.#version += 1; + if (event.key !== this.#key) return; + if (event.newValue === null) return; // maybe PersistedStorage.current should also be deleted? + const newVal = this.#deserialize(event.newValue); + if (newVal !== DESERIALIZATION_ERROR) { + this.#current = newVal; + this.#version += 1; + } }; - #deserialize(value: string): T | undefined { + #deserialize(value: string): T | DESERIALIZATION_ERROR { try { return this.#serializer.deserialize(value); } catch (error) { console.error(`Error when parsing "${value}" from persisted store "${this.#key}"`, error); - return; + return DESERIALIZATION_ERROR; } } - #serialize(value: T | undefined): void { + #serialize(value: T): void { try { - if (value != undefined) { - this.#storage?.setItem(this.#key, this.#serializer.serialize(value)); - } + this.#storage?.setItem(this.#key, this.#serializer.serialize(value)); } catch (error) { console.error( `Error when writing value from persisted store "${this.#key}" to ${this.#storage}`, From e775d0d2936cf5e384790f4d85d8e1b8cae22b4b Mon Sep 17 00:00:00 2001 From: aster <137767097+aster-void@users.noreply.github.com> Date: Fri, 6 Jun 2025 11:40:14 +0900 Subject: [PATCH 3/4] doc: create changeset --- .changeset/serious-icons-drop.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/serious-icons-drop.md diff --git a/.changeset/serious-icons-drop.md b/.changeset/serious-icons-drop.md new file mode 100644 index 00000000..1b193c62 --- /dev/null +++ b/.changeset/serious-icons-drop.md @@ -0,0 +1,5 @@ +--- +"runed": patch +--- + +fix(PersistedState): do not ignore `null` or `undefined` (breaking) From 52f25043696d860df916af357a87af8810c1f6eb Mon Sep 17 00:00:00 2001 From: aster <137767097+aster-void@users.noreply.github.com> Date: Fri, 6 Jun 2025 11:57:20 +0900 Subject: [PATCH 4/4] style: format code --- .../persisted-state/persisted-state.svelte.ts | 11 +- .../persisted-state.test.svelte.ts | 104 ++++++++---------- 2 files changed, 53 insertions(+), 62 deletions(-) diff --git a/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts b/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts index a10f2395..b114cd57 100644 --- a/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts +++ b/packages/runed/src/lib/utilities/persisted-state/persisted-state.svelte.ts @@ -51,10 +51,13 @@ export class PersistedState { constructor(key: string, initialValue: T, options: PersistedStateOptions = {}) { const { storage: storageType = "local", - serializer = { serialize: JSON.stringify, deserialize: (val) => { - if (val === "undefined") return undefined; // JSON.parse can't parse "undefined", but JSON.stringify will serialize undefined to "undefined" - return JSON.parse(val); - } }, + serializer = { + serialize: JSON.stringify, + deserialize: (val) => { + if (val === "undefined") return undefined; // JSON.parse can't parse "undefined", but JSON.stringify will serialize undefined to "undefined" + return JSON.parse(val); + }, + }, syncTabs = true, } = options; const window = "window" in options ? options.window : defaultWindow; // window is not mockable to be undefined without this, because JavaScript will fill undefined with `= default` diff --git a/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts b/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts index 7eef104f..480f6f9c 100644 --- a/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts +++ b/packages/runed/src/lib/utilities/persisted-state/persisted-state.test.svelte.ts @@ -58,24 +58,18 @@ describe("PersistedState", async () => { expect(localStorage.getItem(key)).toBe(JSON.stringify(newValue)); }); - testWithEffect( - "updates localStorage when a nested property in current value changes", - () => { - const propValue = "test"; - const initialValue = { prop: { nested: propValue } }; - const newPropValue = "new test"; - const newValue = { prop: { nested: newPropValue } }; - const persistedState = new PersistedState<{ prop: { nested: string } }>( - key, - initialValue, - ); - expect(persistedState.current).toEqual(initialValue); + testWithEffect("updates localStorage when a nested property in current value changes", () => { + const propValue = "test"; + const initialValue = { prop: { nested: propValue } }; + const newPropValue = "new test"; + const newValue = { prop: { nested: newPropValue } }; + const persistedState = new PersistedState<{ prop: { nested: string } }>(key, initialValue); + expect(persistedState.current).toEqual(initialValue); - persistedState.current.prop.nested = newPropValue; - expect(persistedState.current).toEqual(newValue); - expect(localStorage.getItem(key)).toBe(JSON.stringify(newValue)); - }, - ); + persistedState.current.prop.nested = newPropValue; + expect(persistedState.current).toEqual(newValue); + expect(localStorage.getItem(key)).toBe(JSON.stringify(newValue)); + }); testWithEffect("updates current value when localStorage changes", () => { const propValue = "test"; @@ -140,25 +134,22 @@ describe("PersistedState", async () => { }); describe("syncTabs", () => { - testWithEffect( - "updates persisted value when local storage changes independently", - () => { - $effect(() => { - const persistedState = new PersistedState(key, initialValue); - expect(persistedState.current).toBe(initialValue); - - localStorage.setItem(key, JSON.stringify(newValue)); - window.dispatchEvent( - new StorageEvent("storage", { - key, - oldValue: null, - newValue: JSON.stringify(newValue), - }), - ); - expect(persistedState.current).toBe(newValue); - }); - }, - ); + testWithEffect("updates persisted value when local storage changes independently", () => { + $effect(() => { + const persistedState = new PersistedState(key, initialValue); + expect(persistedState.current).toBe(initialValue); + + localStorage.setItem(key, JSON.stringify(newValue)); + window.dispatchEvent( + new StorageEvent("storage", { + key, + oldValue: null, + newValue: JSON.stringify(newValue), + }) + ); + expect(persistedState.current).toBe(newValue); + }); + }); testWithEffect( "does not update persisted value when local storage changes independently if syncTabs is false", @@ -175,33 +166,30 @@ describe("PersistedState", async () => { key, oldValue: null, newValue: JSON.stringify(newValue), - }), + }) ); expect(persistedState.current).toBe(initialValue); }); - }, + } ); - testWithEffect( - "does not handle the storage event when 'session' storage is used", - () => { - $effect(() => { - const persistedState = new PersistedState(key, initialValue, { - storage: "session", - }); - expect(persistedState.current).toBe(initialValue); - - sessionStorage.setItem(key, JSON.stringify(newValue)); - window.dispatchEvent( - new StorageEvent("storage", { - key, - oldValue: null, - newValue: JSON.stringify(newValue), - }), - ); - expect(persistedState.current).toBe(initialValue); + testWithEffect("does not handle the storage event when 'session' storage is used", () => { + $effect(() => { + const persistedState = new PersistedState(key, initialValue, { + storage: "session", }); - }, - ); + expect(persistedState.current).toBe(initialValue); + + sessionStorage.setItem(key, JSON.stringify(newValue)); + window.dispatchEvent( + new StorageEvent("storage", { + key, + oldValue: null, + newValue: JSON.stringify(newValue), + }) + ); + expect(persistedState.current).toBe(initialValue); + }); + }); }); });