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): fix hydration flicker on initial render of ssr page #33134

Merged
merged 2 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,22 @@ describe(`build-headers-program`, () => {
pluginCreatorId: `049c1cfd-95f7-5555-a4ac-9b396d098b26`,
},
],
[
`/test/`,
{
jsonName: `test`,
internalComponentName: `ComponentTest`,
path: `/test/`,
matchPath: undefined,
componentChunkName: `component---src-pages-test-js`,
isCreatedByStatefulCreatePages: true,
context: {},
updatedAt: 1557740602361,
pluginCreator___NODE: `049c1cfd-95f7-5555-a4ac-9b396d098b26`,
pluginCreatorId: `049c1cfd-95f7-5555-a4ac-9b396d098b26`,
mode: `SSR`,
},
],
[
`/`,
{
Expand Down Expand Up @@ -203,8 +219,8 @@ describe(`build-headers-program`, () => {
expect(output).toMatchSnapshot()
expect(output).toMatch(/app-data\.json/)
expect(output).toMatch(/page-data\.json/)
// we should only check page-data & app-data once which leads to 2 times
expect(fs.existsSync).toBeCalledTimes(2)
// we should only check app-data once which leads to 1 time
expect(fs.existsSync).toBeCalledTimes(1)
})

it(`with manifest['pages-manifest']`, async () => {
Expand Down Expand Up @@ -237,7 +253,7 @@ describe(`build-headers-program`, () => {
expect(output).toMatchSnapshot()
expect(output).toMatch(/\/pages-manifest-ab11f09e0ca7ecd3b43e\.js/g)
expect(output).not.toMatch(/\/app-data\.json/g)
expect(output).not.toMatch(/\/page-data\.json/g)
expect(output).toMatch(/\/page-data\.json/g)
expect(output).not.toMatch(/\/undefined/g)
})

Expand Down
13 changes: 3 additions & 10 deletions packages/gatsby-plugin-gatsby-cloud/src/build-headers-program.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,6 @@ function preloadHeadersByPage({ pages, manifest, pathPrefix, publicFolder }) {
const appDataPath = publicFolder(PAGE_DATA_DIR, `app-data.json`)
const hasAppData = existsSync(appDataPath)

let hasPageData = false
if (pages.size) {
// test if 1 page-data file exists, if it does we know we're on a gatsby version that supports page-data
const pageDataPath = publicFolder(
getPageDataPath(pages.get(pages.keys().next().value).path)
)
hasPageData = existsSync(pageDataPath)
}

Comment on lines -103 to -111
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This heuristic is no longer working as intended with non-ssg pages and it was there to support really old gatsby version (early v2?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed

pages.forEach(page => {
const scripts = _.flatMap(COMMON_BUNDLES, file =>
getScriptPath(file, manifest)
Expand All @@ -121,7 +112,9 @@ function preloadHeadersByPage({ pages, manifest, pathPrefix, publicFolder }) {
json.push(posix.join(PAGE_DATA_DIR, `app-data.json`))
}

if (hasPageData) {
// page-data gets inline for SSR, so we won't be doing page-data request
// and we shouldn't add preload link header for it.
if (page.mode !== `SSR`) {
json.push(getPageDataPath(page.path))
}

Expand Down
10 changes: 9 additions & 1 deletion packages/gatsby/cache-dir/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ const createComponentUrls = componentChunkName =>
)

export class ProdLoader extends BaseLoader {
constructor(asyncRequires, matchPaths) {
constructor(asyncRequires, matchPaths, pageData) {
const loadComponent = chunkName => {
if (!asyncRequires.components[chunkName]) {
throw new Error(
Expand All @@ -504,6 +504,14 @@ export class ProdLoader extends BaseLoader {
}

super(loadComponent, matchPaths)

if (pageData) {
this.pageDataDb.set(pageData.path, {
pagePath: pageData.path,
payload: pageData,
status: `success`,
})
}
}

doPrefetch(pagePath) {
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/cache-dir/production-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import stripPrefix from "./strip-prefix"
// Generated during bootstrap
import matchPaths from "$virtual/match-paths.json"

const loader = new ProdLoader(asyncRequires, matchPaths)
const loader = new ProdLoader(asyncRequires, matchPaths, window.pageData)
setLoader(loader)
loader.setApiRunner(apiRunner)

Expand Down
7 changes: 5 additions & 2 deletions packages/gatsby/cache-dir/static-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export default async function staticPage({
scripts,
reversedStyles,
reversedScripts,
inlinePageData = false,
}) {
// for this to work we need this function to be sync or at least ensure there is single execution of it at a time
global.unsafeBuiltinUsage = []
Expand Down Expand Up @@ -330,7 +331,7 @@ export default async function staticPage({
)
})

if (pageData) {
if (pageData && !inlinePageData) {
headComponents.push(
<link
as="fetch"
Expand Down Expand Up @@ -393,7 +394,9 @@ export default async function staticPage({
})

// Add page metadata for the current page
const windowPageData = `/*<![CDATA[*/window.pagePath="${pagePath}";/*]]>*/`
const windowPageData = `/*<![CDATA[*/window.pagePath="${pagePath}";${
inlinePageData ? `window.pageData=${JSON.stringify(pageData)};` : ``
}/*]]>*/`

postBodyComponents.push(
<script
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby/src/utils/page-ssr-module/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export async function renderHTML({
pageData,
staticQueryContext,
...data.templateDetails.assets,
inlinePageData: data.page.mode === `SSR` && data.results.serverData,
})

return results.html
Expand Down