-
Notifications
You must be signed in to change notification settings - Fork 42
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
Simplify createDerivedStore in debug.derived.ts #1801
base: main
Are you sure you want to change the base?
Conversation
…t expose `set` and `update`.
bdd4b9b
to
0afb9d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have tests for this feature.
Did you try that it worked by:
- Go to Wallet page.
- Click six times in the header.
- Enter "fo" in the prompt.
- Does the JSON file have the data of the "selectedAccount"?
Actually, I forgot to bind the method to the object so I expect it won't work. |
Should this PR be closed ? |
@dskloetd what do you think? |
Does it matter? It's marked as Draft already. |
Motivation
Small optimization.
If all we want is a Readable view on a writable store, that doesn't expose
set
andupdate
, creating a derived store is overkill.The derived store subscribes to the original and forwards the changes but this is not necessary.
Changes
Change createDerivedStore to simply return an object with the same subscribe method and no other methods.
Tests
Refactoring should be covered by existing tests.