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

Allow setting custom storage engine per atom #30

Merged
merged 5 commits into from
Apr 13, 2023
Merged

Allow setting custom storage engine per atom #30

merged 5 commits into from
Apr 13, 2023

Conversation

kytta
Copy link
Contributor

@kytta kytta commented Mar 27, 2023

Closes #27

This is my shot at implementing a custom engine per atom. My particular use case: using one store in localStorage and the other in sessionStorage.

@kytta
Copy link
Contributor Author

kytta commented Mar 27, 2023

Todos for this PR:

  • update index.js
  • update index.d.ts
  • write tests
  • size-limit fails; either increase the limit or try to compact the code

@ai
Copy link
Member

ai commented Mar 28, 2023

size-limit fails; either increase the limit or try to compact the code

Update limit to track the current progress in diff.

index.d.ts Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@ai
Copy link
Member

ai commented Apr 3, 2023

Looks good. Rebase to new main, write test, and we can merge it.

@kytta
Copy link
Contributor Author

kytta commented Apr 5, 2023

I have just noticed, that I forgot about persistentMap; I think we should add the custom storage there too, since it also uses the PersistentOptions type

@@ -43,6 +43,21 @@ test('saves to localStorage', () => {
equal(events, ['1', undefined])
})

test('saves to a custom storage', () => {
Copy link
Member

Choose a reason for hiding this comment

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

We need to test that didn’t change default storage engine

@ai ai marked this pull request as ready for review April 13, 2023 21:28
@ai ai merged commit 1cf7926 into nanostores:main Apr 13, 2023
@ai
Copy link
Member

ai commented Apr 13, 2023

I tried to merge it and fix persistentMap but I found that you're changing only storageEngine. But we need also to change eventEngine.

Maybe we need another API for that.

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.

Question: custom persistent storage per persistentAtom
2 participants