Skip to content

Commit a0bba64

Browse files
vtenfyspieh
authored andcommitted
refactor(gatsby): improve EnsureResources (#10224)
- `getDerivedStateFromProps` - Remove unused parameter `pageResources` - Return even if we don't have resources, to allow detecting them later - Remove `componentDidUpdate` and move some of its contents into new `retryResources` - Throw an error on initial render only upon missing resources - otherwise just don't update the component, and remove the function `shouldRenderStaticHTML` - Note: this may need updating in the future - React docs say `shouldComponentUpdate` is only for performance purposes and may not prevent rendering in future versions - Add a new function for detecting if we have resources which works for production and develop
1 parent d800aae commit a0bba64

File tree

2 files changed

+61
-67
lines changed

2 files changed

+61
-67
lines changed

packages/gatsby/cache-dir/ensure-resources.js

Lines changed: 60 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import PropTypes from "prop-types"
33
import loader from "./loader"
44
import shallowCompare from "shallow-compare"
55

6+
let isInitialRender = true
7+
68
// Pass pathname in as prop.
79
// component will try fetching resources. If they exist,
810
// will just render, else will render null.
@@ -19,53 +21,86 @@ class EnsureResources extends React.Component {
1921
}
2022
}
2123

22-
static getDerivedStateFromProps({ pageResources, location }, prevState) {
24+
reloadPage(prevHref) {
25+
// Do this, rather than simply `window.location.reload()`, so that
26+
// pressing the back/forward buttons work - otherwise when pressing
27+
// back, the browser will just change the URL and expect JS to handle
28+
// the change, which won't always work since it might not be a Gatsby
29+
// page.
30+
const { href } = window.location
31+
window.history.replaceState({}, ``, prevHref)
32+
window.location.replace(href)
33+
}
34+
35+
static getDerivedStateFromProps({ location }, prevState) {
2336
if (prevState.location !== location) {
2437
const pageResources = loader.getResourcesForPathnameSync(
2538
location.pathname
2639
)
2740

28-
if (pageResources) {
29-
return {
30-
pageResources,
31-
location: { ...location },
32-
}
41+
return {
42+
pageResources,
43+
location: { ...location },
3344
}
3445
}
3546

3647
return null
3748
}
3849

39-
componentDidUpdate(prevProps) {
40-
if (prevProps === this.props) return
50+
hasResources(pageResources) {
51+
if (pageResources && pageResources.json) {
52+
return true
53+
}
4154

42-
const { pathname } = this.props.location
55+
if (pageResources && process.env.NODE_ENV !== `production`) {
56+
return true
57+
}
58+
59+
return false
60+
}
61+
62+
retryResources(nextProps) {
63+
const { pathname } = nextProps.location
64+
65+
if (!loader.getResourcesForPathnameSync(pathname)) {
66+
// Store the previous and next location before resolving resources
67+
const prevLocation = this.props.location
68+
this.nextLocation = nextProps.location
4369

44-
if (!loader.getResourcesForPathnameSync(pathname))
4570
// Page resources won't be set in cases where the browser back button
4671
// or forward button is pushed as we can't wait as normal for resources
4772
// to load before changing the page.
4873
loader.getResourcesForPathname(pathname).then(pageResources => {
4974
// The page may have changed since we started this, in which case doesn't update
75+
if (this.nextLocation !== nextProps.location) {
76+
return
77+
}
5078

51-
if (this.props.location.pathname !== location.pathname) {
79+
if (this.hasResources(pageResources)) {
80+
this.setState({
81+
location: { ...window.location },
82+
pageResources,
83+
})
5284
return
5385
}
5486

55-
this.setState({
56-
location: { ...location },
57-
pageResources,
58-
})
87+
// If we still don't have resources, reload the page.
88+
// (This won't happen on initial render, since shouldComponentUpdate
89+
// is only called when the component updates.)
90+
this.reloadPage(prevLocation.href)
5991
})
92+
}
6093
}
6194

6295
shouldComponentUpdate(nextProps, nextState) {
63-
// 404
64-
if (!nextState.pageResources) {
65-
return true
96+
// Always return false if we're missing resources.
97+
if (!this.hasResources(nextState.pageResources)) {
98+
this.retryResources(nextProps)
99+
return false
66100
}
101+
67102
// Check if the component or json have changed.
68-
if (!this.state.pageResources && nextState.pageResources) {
103+
if (this.state.pageResources !== nextState.pageResources) {
69104
return true
70105
}
71106
if (
@@ -92,54 +127,14 @@ class EnsureResources extends React.Component {
92127
return shallowCompare(this, nextProps, nextState)
93128
}
94129

95-
shouldRenderStaticHTML() {
96-
const { localStorage } = window
97-
const { href, pathname } = window.location
98-
99-
// This should only occur if the network is offline, or if the
100-
// path is nonexistent and there's no custom 404 page.
101-
if (
102-
process.env.NODE_ENV === `production` &&
103-
!(this.state.pageResources && this.state.pageResources.json)
104-
) {
105-
if (localStorage.getItem(`___failedResources`) === pathname) {
106-
// Maybe it will work again in the future, so remove the flag
107-
localStorage.removeItem(`___failedResources`)
108-
console.error(
109-
`WARNING: Resources cannot be loaded for the pathname ${pathname} - ` +
110-
`falling back to static HTML instead.\n` +
111-
`This is likely due to a bug in Gatsby, or misconfiguration in your project.`
112-
)
113-
} else {
114-
// Mark the pathname as failed
115-
localStorage.setItem(`___failedResources`, pathname)
116-
117-
// Reload the page.
118-
// Do this, rather than simply `window.location.reload()`, so that
119-
// pressing the back/forward buttons work - otherwise when pressing
120-
// back, the browser will just change the URL and expect JS to handle
121-
// the change, which won't always work since it might not be a Gatsby
122-
// page.
123-
const originalUrl = new URL(href)
124-
window.history.replaceState({}, `404`, `${pathname}?gatsby-404`)
125-
window.location.replace(originalUrl)
126-
}
127-
128-
return true
129-
} else {
130-
localStorage.removeItem(`___failedResources`)
131-
return false
132-
}
133-
}
134-
135130
render() {
136-
// TODO: find a nicer way to do this (e.g. React Suspense)
137-
if (this.shouldRenderStaticHTML()) {
138-
const __html = document.getElementById(`___gatsby`).innerHTML
139-
return <div dangerouslySetInnerHTML={{ __html }} />
140-
} else {
141-
return this.props.children(this.state)
131+
if (!this.hasResources(this.state.pageResources) && isInitialRender) {
132+
// prevent hydrating
133+
throw new Error(`Missing resources for ${this.state.location.pathname}`)
142134
}
135+
136+
isInitialRender = false
137+
return this.props.children(this.state)
143138
}
144139
}
145140

packages/gatsby/cache-dir/loader.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ const fetchResource = resourceName => {
8989
const fetchPromise = resourceFunction()
9090
let failed = false
9191
return fetchPromise
92-
.catch(e => {
93-
console.error(e)
92+
.catch(() => {
9493
failed = true
9594
})
9695
.then(component => {

0 commit comments

Comments
 (0)