Skip to content

Conversation

@gibkigonzo
Copy link
Contributor

@gibkigonzo gibkigonzo commented Nov 19, 2019

Related issues

closes #3669

Short description and why it's useful

Problem occur because we mount app before async route component is resolved. So we can use beforeResolve. As described in vue-router docs it's triggered "after all in-component guards and async route components are resolved". We only need to check if app is already loaded.

QA

  1. Change in src/themes/default/router/index.js
const Home = () => import(/* webpackChunkName: "vsf-home" */ 'theme/pages/Home.vue')

to

const Home = () => new Promise((resolve) => {
  setTimeout(async () => {
    resolve(import(/* webpackChunkName: "vsf-home" */ 'theme/pages/Home.vue'))
  }, 5000)
})

This will simulate very long lazy loading.
2. check homepage in dev and prod mode => it should crash in prod and there should be mismatch in dev mode

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility and no breaking changes)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

@gibkigonzo gibkigonzo changed the base branch from master to hotfix/v1.10.5 November 19, 2019 13:10
@pkarw pkarw requested a review from andrzejewsky November 20, 2019 07:27
if (!from.name) return next() // do not resolve asyncData on server render - already been done
if (!from.name) {
next()
if (canBeMounted()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be executed before next()?

Copy link
Contributor Author

@gibkigonzo gibkigonzo Nov 20, 2019

Choose a reason for hiding this comment

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

Problem is that vue-router doesn't reveal when it resolve async components. So I check when this.pending is falsy, because it gets null just after enter events are finished (here). In this line it adds this.router.resolveHooks (beforeResolve) to other enter events. So we need to wait just after beforeResolve is finished and then we have falsy this.pending. Best solution would be mounting app after queue ends (here) in onReady, but I couldn't find any flag to check this before it goes to beforeResolve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to keep app mounting in onReady callback because in production version on for example homepage there could be a situation that beforeResolve event is not triggered at all.

@pkarw pkarw merged commit c5462b5 into vuestorefront:hotfix/v1.10.5 Nov 21, 2019
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