-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Bugfix/multiple local cache load for wishlist and compare #2431
Bugfix/multiple local cache load for wishlist and compare #2431
Conversation
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.
@patzick the question is why “load” action is called multiple times? Isn’t it strange?
If this is the only ultimate solution then we should add the same to “cart” module as well :) |
@pkarw that's because But i don't get it why we should now add it to the cart module as well. |
Ok, now I got it. It would be much better to have more general solution maybe we can deal with this in this general mixin/ having Singleton like behavior or whatever? I mean: it’s easy to forget about this flagging when creating a new module like compare - our core modules should be kind of “best practices” and this solution looks a little bit like workaround than best practice :-) Wdyt? |
Yeah i thought about it a lot, but it's not that obvious. My first thought was to just add mounted on App.vue component and then we could create hook/event/whatever to just connect to it. But unfortunately when i tested it mounted event on App doesn't mean that other components are rendered too, so it called this to early and either way we had ssr mismatch problem. |
In that case, we should at least create some hook (I mean - re-usable behavior like React hooks or mixin) or whatever to simply have a tool for further modules like this something You can invoke in Your Vuex action to make sure the logic is executed just once - something like Singleton / Mutex in multithreading Maybe we could use AsyncDataLoader for this purpose adding sth like AsyncDataLoader.once() This is just an idea. Please fix this issue like You were doing framework not like a case-by-case hotfix :) |
@pkarw imo the problem is not about invoking it multiple times but about invoking it after hydration. We just need a way to know when it happens (@patzick is already trying to figure it out). Once we figure it out we will think about proper API to introduce it (probably as a part of before/after hook signature as you suggested like |
Hydration is executed in the client-entry ($mount). |
@pkarw when we discover a way we'll make hook for this. Currently i believe this is the best way to deal with this problem. Vue 2.6 "serverPrefetch" is not solving this issue - it just allows to do asyncData in any component, so still you have no access to localStorage and will break hydration calling it there. P.S. i've also tried your $mounted idea with no positive results |
@patzick maybe let's keep the PR open and figure out the API for a proper hook before the stabilization phase. I have some ideas, let's discuss them ;) |
I have response from Vue core team. Evan said that currently there is no way to detect that hydration is completed. His suggestion is to add changes to components - static and lazy loaded. |
Ok! |
Short description and why it's useful
Ensure that wishlist and compare module read only once from local cache.
Screenshots of visual changes before/after (if there are any)
Problem happened on product page, because there is couple of wishlist and compare components.
Before:
After:
Upgrade Notes and Changelog