From e1cef8d1f1e9fda7e833a854b51f6cb8fa81223e Mon Sep 17 00:00:00 2001 From: David Bailey Date: Fri, 30 Nov 2018 17:24:36 +0000 Subject: [PATCH 1/6] fix(gatsby): refactor EnsureResources --- packages/gatsby/cache-dir/ensure-resources.js | 94 ++++++------------- 1 file changed, 31 insertions(+), 63 deletions(-) diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 53b611c0aa4ba..66412a2a2c348 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -19,53 +19,62 @@ class EnsureResources extends React.Component { } } - static getDerivedStateFromProps({ pageResources, location }, prevState) { + static getDerivedStateFromProps({ location }, prevState) { if (prevState.location !== location) { const pageResources = loader.getResourcesForPathnameSync( location.pathname ) - if (pageResources) { - return { - pageResources, - location: { ...location }, - } + return { + pageResources, + location: { ...location }, } } return null } - componentDidUpdate(prevProps) { - if (prevProps === this.props) return + retryResources(nextProps) { + const { pathname } = nextProps.location - const { pathname } = this.props.location + if (!loader.getResourcesForPathnameSync(pathname)) { + // Store the next location before resolving resources to check for updates + this.nextLocation = nextProps.location - if (!loader.getResourcesForPathnameSync(pathname)) // Page resources won't be set in cases where the browser back button // or forward button is pushed as we can't wait as normal for resources // to load before changing the page. loader.getResourcesForPathname(pathname).then(pageResources => { // The page may have changed since we started this, in which case doesn't update + if (this.nextLocation !== nextProps.location) { + return + } - if (this.props.location.pathname !== location.pathname) { + if (pageResources && pageResources.json) { + this.setState({ + location: { ...location }, + pageResources, + }) return } - this.setState({ - location: { ...location }, - pageResources, - }) + // If we still don't have resources, reload the page. + // (This won't happen on initial render, since shouldComponentUpdate + // isn't called for initial renders.) + window.location.reload() }) + } } shouldComponentUpdate(nextProps, nextState) { - // 404 - if (!nextState.pageResources) { - return true + // Always return false if we're missing resources. + if (!(nextState.pageResources && nextState.pageResources.json)) { + this.retryResources(nextProps) + return false } + // Check if the component or json have changed. - if (!this.state.pageResources && nextState.pageResources) { + if (this.state.pageResources !== nextState.pageResources) { return true } if ( @@ -92,53 +101,12 @@ class EnsureResources extends React.Component { return shallowCompare(this, nextProps, nextState) } - shouldRenderStaticHTML() { - const { localStorage } = window - const { href, pathname } = window.location - - // This should only occur if the network is offline, or if the - // path is nonexistent and there's no custom 404 page. - if ( - process.env.NODE_ENV === `production` && - !(this.state.pageResources && this.state.pageResources.json) - ) { - if (localStorage.getItem(`___failedResources`) === pathname) { - // Maybe it will work again in the future, so remove the flag - localStorage.removeItem(`___failedResources`) - console.error( - `WARNING: Resources cannot be loaded for the pathname ${pathname} - ` + - `falling back to static HTML instead.\n` + - `This is likely due to a bug in Gatsby, or misconfiguration in your project.` - ) - } else { - // Mark the pathname as failed - localStorage.setItem(`___failedResources`, pathname) - - // Reload the page. - // Do this, rather than simply `window.location.reload()`, so that - // pressing the back/forward buttons work - otherwise when pressing - // back, the browser will just change the URL and expect JS to handle - // the change, which won't always work since it might not be a Gatsby - // page. - const originalUrl = new URL(href) - window.history.replaceState({}, `404`, `${pathname}?gatsby-404`) - window.location.replace(originalUrl) - } - - return true - } else { - localStorage.removeItem(`___failedResources`) - return false - } - } - render() { - // TODO: find a nicer way to do this (e.g. React Suspense) - if (this.shouldRenderStaticHTML()) { + if (this.state.pageResources && this.state.pageResources.json) { + return this.props.children(this.state) + } else { const __html = document.getElementById(`___gatsby`).innerHTML return
- } else { - return this.props.children(this.state) } } } From 0eb19751e24f678c5aee646b0bd2d74657e28e85 Mon Sep 17 00:00:00 2001 From: David Bailey Date: Fri, 30 Nov 2018 18:19:58 +0000 Subject: [PATCH 2/6] Re-add reload workaround --- packages/gatsby/cache-dir/ensure-resources.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 66412a2a2c348..5b9eb92c2edd5 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -19,6 +19,17 @@ class EnsureResources extends React.Component { } } + reloadPage(prevHref) { + // Do this, rather than simply `window.location.reload()`, so that + // pressing the back/forward buttons work - otherwise when pressing + // back, the browser will just change the URL and expect JS to handle + // the change, which won't always work since it might not be a Gatsby + // page. + const { href } = window.location + window.history.replaceState({}, ``, prevHref) + window.location.replace(href) + } + static getDerivedStateFromProps({ location }, prevState) { if (prevState.location !== location) { const pageResources = loader.getResourcesForPathnameSync( @@ -38,7 +49,8 @@ class EnsureResources extends React.Component { const { pathname } = nextProps.location if (!loader.getResourcesForPathnameSync(pathname)) { - // Store the next location before resolving resources to check for updates + // Store the previous and next location before resolving resources + const prevLocation = this.props.location this.nextLocation = nextProps.location // Page resources won't be set in cases where the browser back button @@ -52,7 +64,7 @@ class EnsureResources extends React.Component { if (pageResources && pageResources.json) { this.setState({ - location: { ...location }, + location: { ...window.location }, pageResources, }) return @@ -61,7 +73,7 @@ class EnsureResources extends React.Component { // If we still don't have resources, reload the page. // (This won't happen on initial render, since shouldComponentUpdate // isn't called for initial renders.) - window.location.reload() + this.reloadPage(prevLocation.href) }) } } From 5130484b4a71c5a2e66240f92416ef77650a370e Mon Sep 17 00:00:00 2001 From: David Bailey Date: Fri, 30 Nov 2018 21:43:48 +0000 Subject: [PATCH 3/6] Fix gatsby develop not working --- packages/gatsby/cache-dir/ensure-resources.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 5b9eb92c2edd5..565e89256b5c4 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -80,7 +80,10 @@ class EnsureResources extends React.Component { shouldComponentUpdate(nextProps, nextState) { // Always return false if we're missing resources. - if (!(nextState.pageResources && nextState.pageResources.json)) { + if ( + process.env.NODE_ENV === `production` && + !(nextState.pageResources && nextState.pageResources.json) + ) { this.retryResources(nextProps) return false } @@ -114,7 +117,10 @@ class EnsureResources extends React.Component { } render() { - if (this.state.pageResources && this.state.pageResources.json) { + if ( + process.env.NODE_ENV !== `production` || + (this.state.pageResources && this.state.pageResources.json) + ) { return this.props.children(this.state) } else { const __html = document.getElementById(`___gatsby`).innerHTML From f506910c36f3d7355bffe1f0b17736879bb83b1a Mon Sep 17 00:00:00 2001 From: David Bailey Date: Sat, 1 Dec 2018 20:35:24 +0000 Subject: [PATCH 4/6] Detect resources properly in develop --- packages/gatsby/cache-dir/ensure-resources.js | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 565e89256b5c4..076919dd01755 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -45,6 +45,18 @@ class EnsureResources extends React.Component { return null } + hasResources(pageResources) { + if (pageResources && pageResources.json) { + return true + } + + if (pageResources && process.env.NODE_ENV !== `production`) { + return true + } + + return false + } + retryResources(nextProps) { const { pathname } = nextProps.location @@ -62,7 +74,7 @@ class EnsureResources extends React.Component { return } - if (pageResources && pageResources.json) { + if (this.hasResources(pageResources)) { this.setState({ location: { ...window.location }, pageResources, @@ -72,7 +84,7 @@ class EnsureResources extends React.Component { // If we still don't have resources, reload the page. // (This won't happen on initial render, since shouldComponentUpdate - // isn't called for initial renders.) + // is only called when the component updates.) this.reloadPage(prevLocation.href) }) } @@ -80,10 +92,7 @@ class EnsureResources extends React.Component { shouldComponentUpdate(nextProps, nextState) { // Always return false if we're missing resources. - if ( - process.env.NODE_ENV === `production` && - !(nextState.pageResources && nextState.pageResources.json) - ) { + if (!this.hasResources(nextState.pageResources)) { this.retryResources(nextProps) return false } @@ -117,10 +126,7 @@ class EnsureResources extends React.Component { } render() { - if ( - process.env.NODE_ENV !== `production` || - (this.state.pageResources && this.state.pageResources.json) - ) { + if (this.hasResources(this.state.pageResources)) { return this.props.children(this.state) } else { const __html = document.getElementById(`___gatsby`).innerHTML From de8a933fcea059771acf7abc0b94139abb8baab3 Mon Sep 17 00:00:00 2001 From: David Bailey Date: Mon, 10 Dec 2018 15:27:10 +0000 Subject: [PATCH 5/6] Throw error to prevent hydrating + don't log undefined --- packages/gatsby/cache-dir/ensure-resources.js | 4 ++-- packages/gatsby/cache-dir/loader.js | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 076919dd01755..e7a6b824e676f 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -129,8 +129,8 @@ class EnsureResources extends React.Component { if (this.hasResources(this.state.pageResources)) { return this.props.children(this.state) } else { - const __html = document.getElementById(`___gatsby`).innerHTML - return
+ // prevent hydrating + throw new Error(`Missing resources for ${this.state.location.pathname}`) } } } diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js index 23a3dde924a7e..f0022ee681e47 100644 --- a/packages/gatsby/cache-dir/loader.js +++ b/packages/gatsby/cache-dir/loader.js @@ -89,8 +89,7 @@ const fetchResource = resourceName => { const fetchPromise = resourceFunction() let failed = false return fetchPromise - .catch(e => { - console.error(e) + .catch(() => { failed = true }) .then(component => { From a504246a009c25df62b9c9ec5f0c6517305ab9af Mon Sep 17 00:00:00 2001 From: David Bailey Date: Mon, 10 Dec 2018 15:48:04 +0000 Subject: [PATCH 6/6] Add check for initial render --- packages/gatsby/cache-dir/ensure-resources.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index e7a6b824e676f..7b2f38244c043 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -3,6 +3,8 @@ import PropTypes from "prop-types" import loader from "./loader" import shallowCompare from "shallow-compare" +let isInitialRender = true + // Pass pathname in as prop. // component will try fetching resources. If they exist, // will just render, else will render null. @@ -126,12 +128,13 @@ class EnsureResources extends React.Component { } render() { - if (this.hasResources(this.state.pageResources)) { - return this.props.children(this.state) - } else { + if (!this.hasResources(this.state.pageResources) && isInitialRender) { // prevent hydrating throw new Error(`Missing resources for ${this.state.location.pathname}`) } + + isInitialRender = false + return this.props.children(this.state) } }