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

Handle try catch on parse #245

Closed
asjir opened this issue Mar 20, 2024 · 2 comments
Closed

Handle try catch on parse #245

asjir opened this issue Mar 20, 2024 · 2 comments

Comments

@asjir
Copy link

asjir commented Mar 20, 2024

Currently changing the serializer (from json to devalue in my case) is messy, because as long as someone can have stale data, you need to do this (I'm also posting in case other people face this problem):

const browser = typeof window !== "undefined" && typeof document !== "undefined"  // for SSR
if (browser)
  try {
    serializer.parse(localStorage.get(key))
  } catch {
    localStorage.setItem(key, "")  # removeItem didnt work for me
  }
store = persisted(key, defaults, { serializer })

And I found the fact that onError only catches read errors and not parse errors misleading which cost me extra time.

Both these problems would be solved by simply adding another option
onParseError defaulting to rethrowing, that you could supply with your default value - then autocomplete would also make it clear that onError doesn't include parsing errors.

Perhaps onParseError could be passed the serialised string to have the option open for falling back to the old parser, but I wouldn't have used it.

Edit:

I settled on:

import { persisted as _persisted, type Options } from "svelte-persisted-store"
import type { Writable } from "svelte/store"

export const persisted = <T>(key: string, d: T, o?: Options<T>): Writable<T> => {
    try {
        return _persisted(key, d, o)
    } catch {
        localStorage.setItem(key, "")
        return _persisted(key, d, o)
    }
} 
@bertmad3400
Copy link
Contributor

bertmad3400 commented Apr 18, 2024

Is fixed by #246, just awaiting merge

Edit: Is now fixed and available, see readme for syntax

@joshnuss
Copy link
Owner

Closing, as @bertmad3400 mentioned, there is now a solution for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants