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

feat(types): turn LocalForageWrapper into a generic class #528

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

charpeni
Copy link

LocalForageWrapper is currently typed as string | object | null, but it solely depends on whether we serialize or not.

Also, localForage (and IndexedDB) supports storing more than string or object, it supports the following JS objects:

  • Array
  • ArrayBuffer
  • Blob
  • Float32Array
  • Float64Array
  • Int8Array
  • Int16Array
  • Int32Array
  • Number
  • Object
  • Uint8Array
  • Uint8ClampedArray
  • Uint16Array
  • Uint32Array
  • String

I didn't want to introduce a breaking change by changing the type, so instead, I turned LocalForageWrapper into a generic class expecting a type for LocalForage and defaulting to string | object | null (current value).

It means that if you're storing binary data in IndexedDB, you can provide the type as such: LocalForageWrapper<Uint8Array>.

@@ -26,12 +27,9 @@ export class LocalForageWrapper
}
}

interface LocalForageInterface {
export interface LocalForageInterface<T = string | object | null> {
Copy link
Author

Choose a reason for hiding this comment

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

I exported LocalForageInterface here as it could be useful.

I'm currently writing a wrapper for LocalForageWrapper that compresses the cache into a binary, so I need both the generic class and the interface here because I need to use composition over inheritance to be able to turn LocalForageWrapper into PersistentStorage<Uint8Array>, but make sure the newly created wrapper exposes string.

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

Successfully merging this pull request may close these issues.

1 participant