-
Notifications
You must be signed in to change notification settings - Fork 35
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
asyncDerived stores doesn't update when dependencies are Set or Map #68
Comments
could you reproduce this in a code sandbox please? With some comments as to the expected and actual values you are seeing? You've got a bunch of different types of stores in your code snippet, so just including what's necessary to reproduce the part that isn't working would be helpful. |
Hi @Akolyte01 there's the reproduction in code sandbox |
Thanks for that. I see the problem.... Sets and Maps are not stringifyable so the change detection gets confused. I have an idea for how to fix this but I think it might lead to some major refactoring that could take a bit. Thanks for bringing this to my attention! |
Hi @Akolyte01, as a first fix you can try to use devalue instead of JSON.stringify(). |
I'm giving this rewrite a shot at the moment, but if I run into complications I'll use that for a hotfix. |
Glad to see this issue brought up, and glad to see it is getting attention (thanks @Akolyte01!). @square/svelte-store has really made data management and reactivity MUCH more manageable in what I would call a pretty complex application I'm working on--awesome work!--but running into this issue made me wonder if I was misunderstanding how store derivation works. Any work-arounds while @Akolyte01 resolves the issue? Just don't use Map? I'm using Maps and Sets all over a big refactor, and while they aren't the only way to go about it, they definitely help avoid duplicating data. |
This is specifically a problem with Set and Map because they do not stringify properly.
Stringifying is the current method used for comparing parent store values for async stores to determine if new asynchronous behavior needs to be run. This contrasts to derived stores, which just reruns the provided mapping function whenever a subscription update happens. This means that a derived store with 3 parents that each load from an initial to a final value will result in the derived store mapping function being run 6 times. This is fine when dealing with synchronous data, but would be detrimental if it resulted in hitting an endpoint 6 times as often as it needed to be. The stringify solution is far from ideal even beyond compatibility with sets or maps because this ends up needing to be called a lot, which adds non insignificant overhead if using stores to track large data structures. So yes, the workaround currently is to not use maps or sets. You could also fork the repo and use hawk93's suggestion to replace the JSON.stringify calls with devalue, if that would be less work for you. However I should have a beta version of v1.1 out soon that should remove the need to stringify data as well as include some other changes like automatic rebouncing for stores. |
Thank you. I did attempt to implement For the time being, I'm removing Maps and Sets from my code and looking forward to v1.1! |
Just wanted to jump back in and say I've been using the Only thing I'm noticing is that using I'm not sure if |
Hey! Good to hear that things are working for you. I think what you are witnessing is the biggest change in functionality with 1.1, and what is primarily holding back release. This change is that if a store loses all subscriptions and then gains a subscriber again, it will always run its self load functionality again. So I suspect your first load is resolving before you follow up with the other load, resulting in the two self loads. You can circumvent this (for now) by keeping a subscription active for the lifecycle that you want to interact with the store. The reason for this ties into the change detection that's at the root of this ticket. When stringifying the parent values to see if a change has occurred, that string could continue to be tracked when the store lost its subscribers, allowing the store to know if any parent subscriptions occurred during the period the store had no subscribers. But with 1.1 this functionality was changed to instead rely on subscription updates more directly. Consider this scenario:
The only way I have thought of to get around this is for async stores to save their parent values when it goes inactive (due to not having any subscribers) and then when it goes active again it can compare the current parent values to the stored values to determine if it needs to load itself again or use the current value of the store. But the problem with this is that this comparison would need to use stringify or equivalent, which just reintroduces the original problem outlined by this ticket. |
Admittedly, I haven't dug into the code (I'll have to take a look!), so I don't know to what degree you are rewriting svelte/store or what you have access to. Off the top of my head, I'm thinking that if a store knows that it should broadcast its own new value, it can store the current timestamp, and when the child updates it can store that same timestamp in a That all may be totally not how derived stores work, so if I get some time this week, I'll dig into the code and see if I get any bright ideas. |
Hi,
in my project I had a writable store containing a Map with number as keys, that map is a dependency of an asyncDerived store, but I realized my derived store was not triggering the update.
I tried to compare it with the svelte "standard" stores and it works
The text was updated successfully, but these errors were encountered: