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 - revert init filters #3968
bugfix - revert init filters #3968
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.
@gibkigonzo it will cause memory leak - there is a known issue with Vue.mixins and Vue.filters and memory leaks
if we're to move it out this once
helper then it must be executed ONLY in CSR mode - however not sure if it won't cause another side effects blinking or CSR/SSR DOM mismatch
Reference: vuejs/vue#5089
I just reverted this from 1.10.5. It's wierd problem because if we call those initialization in |
@pkarw Problem is that we add filters once and then in |
Or we could pass storeView to price filter (and date filter). Filters would be more pure, but we would have to pass this every time in template |
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.
That makes sense. I'd opt for both solutions:
- use the
global
for thestoreView
placeholder to keep b.c. - use it as a filter parameter (with default value pointing to this global) - to be used in the new themes
export function currentStoreView (): StoreView { | ||
// TODO: Change to getter all along our code | ||
return rootStore.state.storeView | ||
const serverStoreView = get(global, 'process.storeView', undefined) |
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.
what about client-side? there is no global?
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.
there is no global on client-side, I could use window, but I think we can stay with same behavior as it was for client
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.
it is in next line const clientStoreView = get(rootStore, 'state.storeView', undefined)
@gibkigonzo one comment left. Is it ready? |
@pkarw @andrzejewsky I've applied suggestion |
Short Description and Why It's Useful
On init load https://test.storefrontcloud.io/women/women-20 filters are missing on SSR so we get price without format.
Which Environment This Relates To
Check your case. In case of any doubts please read about Release Cycle
develop
branch and want to merge it back todevelop
release
branch and want to merge it back torelease
hotfix
ormaster
branch and want to merge it back tohotfix
Upgrade Notes and Changelog
IMPORTANT NOTICE - Remember to update
CHANGELOG.md
with description of your changeContribution and Currently Important Rules Acceptance