Skip to content
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

fix(gatsby): prevent unmount wrapPageElement while page-data loading in dev #17111

Merged
merged 3 commits into from Sep 24, 2019

Conversation

@ValeraS
Copy link
Contributor

ValeraS commented Aug 27, 2019

In rare cases, when you go to a page, its data may not be loaded yet, which causes unmounting of wrapPageComponent.

Closes #16489

@ValeraS ValeraS requested a review from gatsbyjs/core as a code owner Aug 27, 2019
@pieh

This comment has been minimized.

Copy link
Contributor

pieh commented Sep 12, 2019

This is interesting, it seems to me we should prevent this earlier. We do have <EnsureResources> ( https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/cache-dir/ensure-resources.js ) which does ensure we don't switch component (or emit onRouteUpdate events) until page is actually ready - see

<EnsureResources location={location}>
{locationAndPageResources => (
<RouteUpdates location={location}>

this works pretty well in production builds, but seems like it doesn't cover development

I think there is regression happening there - loader used there to make sure resources are loaded doesn't seem to handle data, looking now at

loadPage(pagePath) {
const realPath = cleanPath(pagePath)
return super.loadPage(realPath).then(result => {
require(`./socketIo`).getPageData(realPath)
return result
})
}

it seems like we are not waiting for getting data from websocket - it should chain then there - otherwise it creates orphaned promise (in require(./socketIo).getPageData(realPath) code, that will resolve sometime in future, but loadPage itself can resolve actually faster

@ValeraS ValeraS force-pushed the ValeraS:fix/prevent-unmount-wrapPageElement-dev branch from 3f9db4c to 565ca17 Sep 12, 2019
@ValeraS ValeraS force-pushed the ValeraS:fix/prevent-unmount-wrapPageElement-dev branch from 5480fa1 to 103be0f Sep 23, 2019
@ValeraS ValeraS force-pushed the ValeraS:fix/prevent-unmount-wrapPageElement-dev branch from 103be0f to f6b5d9b Sep 23, 2019
@pieh pieh self-assigned this Sep 24, 2019
@pieh
pieh approved these changes Sep 24, 2019
Copy link
Contributor

pieh left a comment

This is great and works like a charm! Thanks @ValeraS!

@pieh pieh merged commit 610b581 into gatsbyjs:master Sep 24, 2019
20 checks passed
20 checks passed
Danger All good
Details
Peril All green. Good on 'ya.
Details
ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_pipeline Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_long_term_caching Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: starters_validate Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node10 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node12 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node8 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_www Your tests passed on CircleCI!
Details
ci/circleci: windows_unit_tests Your tests passed on CircleCI!
Details
cypress: default-group 67 tests passed in 00:39
Details
unit_tests_windows Build #20190923.61 succeeded
Details
@ValeraS ValeraS deleted the ValeraS:fix/prevent-unmount-wrapPageElement-dev branch Sep 25, 2019
@pieh

This comment has been minimized.

Copy link
Contributor

pieh commented Sep 25, 2019

Published in gatsby@2.15.23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.