Skip to content

feat(with-storage-sync): indexeddb support#129

Closed
mzkmnk wants to merge 0 commit intoangular-architects:mainfrom
mzkmnk:feat/add-dexie-support
Closed

feat(with-storage-sync): indexeddb support#129
mzkmnk wants to merge 0 commit intoangular-architects:mainfrom
mzkmnk:feat/add-dexie-support

Conversation

@mzkmnk
Copy link
Contributor

@mzkmnk mzkmnk commented Jan 18, 2025

Issue

#111

Content

In this PR, we modified withStorageSync to support IndexedDB.

Breaking Changes

  • The storage argument of withStorageSync has been renamed to storageType and now accepts one of three strings: localStorage, sessionStorage, or indexedDB.
  • When indexedDB is selected as the storageType, two new arguments, dbName and storeName, are added (where dbName represents the database name and storeName is analogous to a table name in a relational database).
  • The functions clearStorage, readFromStorage, and writeToStorage are now treated as asynchronous.

Sample Code

const TodoIndexedDBSync = signalStore(
  withState<...>({...}),
  withStorageSync(
    {
        storage:'indexedDB',
        dbName:'ngrx-toolkit',
        storeName:'users',
    },
  ),
  withMethods((store) => ({...}))
);

Copy link
Collaborator

@rainerhahnekamp rainerhahnekamp left a comment

Choose a reason for hiding this comment

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

@mzkmnk looks good, but I initially meant that we integrate this into the existing withStorageSync. Like how withGlitchTracking or withDisabledNameIndices is done for withDevtools.

You will very likely have to adapt the code of withStorageSync to make this happen.

The reason for not bringing up a second storage syncer extension is that we can reuse parts of its code (like serialization)

@mzkmnk
Copy link
Contributor Author

mzkmnk commented Jan 19, 2025

@rainerhahnekamp
I’m going to create a withIndexedDB to integrate with withStorageSync, taking inspiration from withDevtools, withGlitchTracking, and withDisabledNameIndices! It might take some time though… :)

@mzkmnk mzkmnk changed the title feat(with-indexeddb-sync): indexeddb support feat(with-storage-sync): indexeddb support Jan 19, 2025
@rainerhahnekamp
Copy link
Collaborator

@mzkmnk

It might take some time though… :)

Sure, if you have some work to delegate to me, let me know.

@audhit
Copy link

audhit commented Jan 20, 2025

@mzkmnk great work mate :) Thanks a lot. Hi @rainerhahnekamp can you please guide me how to use it and which version of the toolkit has this feature? Will it gonna be 19.0.3 ? Please let me know. Thanks in advance 🙏

@rainerhahnekamp
Copy link
Collaborator

@audhit it is still in development. Once @mzkmnk is done, he will push the "Ready for review" button.

@mzkmnk
Copy link
Contributor Author

mzkmnk commented Jan 20, 2025

@rainerhahnekamp
I’ve finished the withIndexedDB method (although I haven’t written any tests yet). I’m planning to do some more refactoring—do you have any advice? Also, do you think it would be acceptable to wrap the read and write methods of withStorageSync in something like Promise.resolve() so that they are handled asynchronously?

@mzkmnk mzkmnk force-pushed the feat/add-dexie-support branch from b677206 to 03298f8 Compare January 21, 2025 13:39
@rainerhahnekamp
Copy link
Collaborator

@mzkmnk big thanks for now. I will check the code asap and i do understand the potential necessity of async methods.

@mzkmnk mzkmnk force-pushed the feat/add-dexie-support branch 4 times, most recently from 653add5 to 607eb58 Compare January 28, 2025 03:38
@mzkmnk
Copy link
Contributor Author

mzkmnk commented Jan 28, 2025

@rainerhahnekamp
I have integrated IndexedDB into withStorageSync. Although all the functions have become asynchronous, I believe the usability hasn’t changed significantly. If it seems acceptable to proceed as is, I plan to update the tests and request a review.

@rainerhahnekamp
Copy link
Collaborator

@mzkmnk Thanks for the update. Once you're ready for the review, let me know. Until then 👍good work

@audhit
Copy link

audhit commented Jan 29, 2025

@rainerhahnekamp I have integrated IndexedDB into withStorageSync. Although all the functions have become asynchronous, I believe the usability hasn’t changed significantly. If it seems acceptable to proceed as is, I plan to update the tests and request a review.

@mzkmnk great work, eagerly waiting for it 👍

@mzkmnk mzkmnk force-pushed the feat/add-dexie-support branch 2 times, most recently from 61b282d to e3cd252 Compare January 30, 2025 00:56
@mzkmnk mzkmnk marked this pull request as ready for review February 1, 2025 01:12
@mzkmnk
Copy link
Contributor Author

mzkmnk commented Feb 1, 2025

@rainerhahnekamp
The code is complete. Please review it :octocat:

@mzkmnk mzkmnk force-pushed the feat/add-dexie-support branch 2 times, most recently from 265654e to bcfb642 Compare February 2, 2025 09:33
@mzkmnk mzkmnk closed this Feb 2, 2025
@mzkmnk mzkmnk force-pushed the feat/add-dexie-support branch from bcfb642 to ed7e28d Compare February 2, 2025 09:34
@mzkmnk
Copy link
Contributor Author

mzkmnk commented Feb 2, 2025

@rainerhahnekamp

change PR #134

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.

3 participants