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

Added DataStore that keep data on browser storage. #18

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

nakajo2011
Copy link

Implemented new LocalDataStore that store data on browser storage using window.localStorage.

Changed LocalDataStore names to MockDataStore for add new LocalDataStore. Before's LocalDataStore store data on memory, so I think the name MockDataStore is more appropriate than LocalDataStore.

Changed `LocalDataStore` names to `MockDataStore` for add new `LocalDataStore`.
Before `LocalDataStore` store data on memory, so I think the name `MockDataStore` is more appropriate than `LocalDataStore`.
Copy link
Contributor

@azuchi azuchi left a comment

Choose a reason for hiding this comment

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

There seems to be a conflict, please resolve it.

yarn.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This repository already has package-lock.json. It is undesirable to have two lock files.

Copy link
Author

Choose a reason for hiding this comment

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

Will remove yarn.lock. thanks.

*
* This DataStore is very simple store that save any data in flush memory.
*/
class MockDataStore {
Copy link
Contributor

Choose a reason for hiding this comment

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

The word Mock is a misnomer because it reminds us of test mocks. MemoryStore or similar would be appropriate.

Copy link
Contributor

@Yamaguchi Yamaguchi left a comment

Choose a reason for hiding this comment

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

When using the local browser storage, there is a limit such as up to 10MB of space, but is this PR considered cases where data is stored beyond that limit?

test/local_data_store.spec.ts Outdated Show resolved Hide resolved
@nakajo2011 nakajo2011 closed this Aug 26, 2023
@nakajo2011 nakajo2011 reopened this Aug 26, 2023
@nakajo2011
Copy link
Author

nakajo2011 commented Aug 26, 2023

When using the local browser storage, there is a limit such as up to 10MB of space, but is this PR considered cases where data is stored beyond that limit?

@Yamaguchi
Thanks show important consider. I added limit to items count those are utxos and keys for not over storage limit.

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