From 70cb5bd7269f5f5031cbad283dc3fc6bb216d532 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 4 Mar 2020 06:58:12 -0600 Subject: [PATCH] Update to make sure preview mode works with getServerSideProps (#10813) * Update to make sure preview mode works with getServerSideProps * Update to only parse previewData in GS(S)P mode --- .../next/next-server/server/next-server.ts | 30 +- packages/next/next-server/server/render.tsx | 67 ++-- packages/next/types/index.d.ts | 2 + .../pages/api/preview.js | 4 + .../pages/api/reset.js | 4 + .../getserversideprops-preview/pages/index.js | 15 + .../getserversideprops-preview/server.js | 41 +++ .../test/index.test.js | 300 ++++++++++++++++++ 8 files changed, 423 insertions(+), 40 deletions(-) create mode 100644 test/integration/getserversideprops-preview/pages/api/preview.js create mode 100644 test/integration/getserversideprops-preview/pages/api/reset.js create mode 100644 test/integration/getserversideprops-preview/pages/index.js create mode 100644 test/integration/getserversideprops-preview/server.js create mode 100644 test/integration/getserversideprops-preview/test/index.test.js diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index d6306959d6fc..a03ca8665491 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -905,6 +905,14 @@ export default class Server { }) } + let previewData: string | false | object | undefined + let isPreviewMode = false + + if (isServerProps || isSSG) { + previewData = tryGetPreviewData(req, res, this.renderOpts.previewProps) + isPreviewMode = previewData !== false + } + // non-spr requests should render like normal if (!isSSG) { // handle serverless @@ -923,7 +931,7 @@ export default class Server { !this.renderOpts.dev ? { revalidate: -1, - private: false, // Leave to user-land caching + private: isPreviewMode, // Leave to user-land caching } : undefined ) @@ -946,25 +954,27 @@ export default class Server { !this.renderOpts.dev ? { revalidate: -1, - private: false, // Leave to user-land caching + private: isPreviewMode, // Leave to user-land caching } : undefined ) return null } - return renderToHTML(req, res, pathname, query, { + const html = await renderToHTML(req, res, pathname, query, { ...components, ...opts, }) - } - const previewData = tryGetPreviewData( - req, - res, - this.renderOpts.previewProps - ) - const isPreviewMode = previewData !== false + if (html && isServerProps && isPreviewMode) { + sendPayload(res, html, 'text/html; charset=utf-8', { + revalidate: -1, + private: isPreviewMode, + }) + } + + return html + } // Compute the SPR cache key const urlPathname = parseUrl(req.url || '').pathname! diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 51bb9c94ac83..c77bccb00371 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -321,7 +321,7 @@ export async function renderToHTML( const isFallback = !!query.__nextFallback delete query.__nextFallback - const isSpr = !!getStaticProps + const isSSG = !!getStaticProps const defaultAppGetInitialProps = App.getInitialProps === (App as any).origGetInitialProps @@ -332,7 +332,7 @@ export async function renderToHTML( const isAutoExport = !hasPageGetInitialProps && defaultAppGetInitialProps && - !isSpr && + !isSSG && !getServerSideProps if ( @@ -355,7 +355,7 @@ export async function renderToHTML( ) } - if (hasPageGetInitialProps && isSpr) { + if (hasPageGetInitialProps && isSSG) { throw new Error(SSG_GET_INITIAL_PROPS_CONFLICT + ` ${pathname}`) } @@ -363,17 +363,17 @@ export async function renderToHTML( throw new Error(SERVER_PROPS_GET_INIT_PROPS_CONFLICT + ` ${pathname}`) } - if (getServerSideProps && isSpr) { + if (getServerSideProps && isSSG) { throw new Error(SERVER_PROPS_SSG_CONFLICT + ` ${pathname}`) } - if (!!getStaticPaths && !isSpr) { + if (!!getStaticPaths && !isSSG) { throw new Error( `getStaticPaths was added without a getStaticProps in ${pathname}. Without getStaticProps, getStaticPaths does nothing` ) } - if (isSpr && pageIsDynamic && !getStaticPaths) { + if (isSSG && pageIsDynamic && !getStaticPaths) { throw new Error( `getStaticPaths is required for dynamic SSG pages and is missing for '${pathname}'.` + `\nRead more: https://err.sh/next.js/invalid-getstaticpaths-value` @@ -414,7 +414,7 @@ export async function renderToHTML( } } if (isAutoExport) renderOpts.autoExport = true - if (isSpr) renderOpts.nextExport = false + if (isSSG) renderOpts.nextExport = false await Loadable.preloadAll() // Make sure all dynamic imports are loaded @@ -471,12 +471,16 @@ export async function renderToHTML( router, ctx, }) + let previewData: string | false | object | undefined - if (isSpr && !isFallback) { + if ((isSSG || getServerSideProps) && !isFallback) { // Reads of this are cached on the `req` object, so this should resolve // instantly. There's no need to pass this data down from a previous // invoke, where we'd have to consider server & serverless. - const previewData = tryGetPreviewData(req, res, previewProps) + previewData = tryGetPreviewData(req, res, previewProps) + } + + if (isSSG && !isFallback) { const data = await getStaticProps!({ ...(pageIsDynamic ? { params: query as ParsedUrlQuery } : undefined), ...(previewData !== false @@ -529,6 +533,27 @@ export async function renderToHTML( ;(renderOpts as any).revalidate = data.revalidate ;(renderOpts as any).pageData = props } + + if (getServerSideProps && !isFallback) { + const data = await getServerSideProps({ + req, + res, + query, + ...(pageIsDynamic ? { params: params as ParsedUrlQuery } : undefined), + ...(previewData !== false + ? { preview: true, previewData: previewData } + : undefined), + }) + + const invalidKeys = Object.keys(data).filter(key => key !== 'props') + + if (invalidKeys.length) { + throw new Error(invalidKeysMsg('getServerSideProps', invalidKeys)) + } + + props.pageProps = data.props + ;(renderOpts as any).pageData = props + } } catch (err) { if (!dev || !err) throw err ctx.err = err @@ -536,26 +561,8 @@ export async function renderToHTML( console.error(err) } - if (getServerSideProps && !isFallback) { - const data = await getServerSideProps({ - req, - res, - ...(pageIsDynamic ? { params: params as ParsedUrlQuery } : undefined), - query, - }) - - const invalidKeys = Object.keys(data).filter(key => key !== 'props') - - if (invalidKeys.length) { - throw new Error(invalidKeysMsg('getServerSideProps', invalidKeys)) - } - - props.pageProps = data.props - ;(renderOpts as any).pageData = props - } - if ( - !isSpr && // we only show this warning for legacy pages + !isSSG && // we only show this warning for legacy pages !getServerSideProps && process.env.NODE_ENV !== 'production' && Object.keys(props?.pageProps || {}).includes('url') @@ -577,7 +584,7 @@ export async function renderToHTML( } // the response might be finished on the getInitialProps call - if (isResSent(res) && !isSpr) return null + if (isResSent(res) && !isSSG) return null const devFiles = buildManifest.devFiles const files = [ @@ -634,7 +641,7 @@ export async function renderToHTML( documentCtx ) // the response might be finished on the getInitialProps call - if (isResSent(res) && !isSpr) return null + if (isResSent(res) && !isSSG) return null if (!docProps || typeof docProps.html !== 'string') { const message = `"${getDisplayName( diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index 0c77c3bedf79..ae8bbe80665d 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -83,6 +83,8 @@ export type GetServerSideProps = (context: { res: ServerResponse params?: ParsedUrlQuery query: ParsedUrlQuery + preview?: boolean + previewData?: any }) => Promise<{ [key: string]: any }> export default next diff --git a/test/integration/getserversideprops-preview/pages/api/preview.js b/test/integration/getserversideprops-preview/pages/api/preview.js new file mode 100644 index 000000000000..b201b0e02b90 --- /dev/null +++ b/test/integration/getserversideprops-preview/pages/api/preview.js @@ -0,0 +1,4 @@ +export default (req, res) => { + res.setPreviewData(req.query) + res.status(200).end() +} diff --git a/test/integration/getserversideprops-preview/pages/api/reset.js b/test/integration/getserversideprops-preview/pages/api/reset.js new file mode 100644 index 000000000000..0434bf5b64f4 --- /dev/null +++ b/test/integration/getserversideprops-preview/pages/api/reset.js @@ -0,0 +1,4 @@ +export default (req, res) => { + res.clearPreviewData() + res.status(200).end() +} diff --git a/test/integration/getserversideprops-preview/pages/index.js b/test/integration/getserversideprops-preview/pages/index.js new file mode 100644 index 000000000000..848b6a14138e --- /dev/null +++ b/test/integration/getserversideprops-preview/pages/index.js @@ -0,0 +1,15 @@ +export function getServerSideProps({ preview, previewData }) { + return { props: { hasProps: true, preview, previewData } } +} + +export default function({ hasProps, preview, previewData }) { + if (!hasProps) { + return 
Has No Props
+ } + + return ( + 
+      {JSON.stringify(preview) + ' and ' + JSON.stringify(previewData)}
+
+ ) +} diff --git a/test/integration/getserversideprops-preview/server.js b/test/integration/getserversideprops-preview/server.js new file mode 100644 index 000000000000..c35347a48951 --- /dev/null +++ b/test/integration/getserversideprops-preview/server.js @@ -0,0 +1,41 @@ +const http = require('http') +const url = require('url') +const fs = require('fs') +const path = require('path') +const server = http.createServer((req, res) => { + let { pathname } = url.parse(req.url) + if (pathname.startsWith('/_next/data')) { + pathname = pathname + .replace(`/_next/data/${process.env.BUILD_ID}/`, '/') + .replace(/\.json$/, '') + } + console.log('serving', pathname) + + if (pathname === '/favicon.ico') { + res.statusCode = 404 + return res.end() + } + + if (pathname.startsWith('/_next/static/')) { + res.write( + fs.readFileSync( + path.join( + __dirname, + './.next/static/', + pathname.slice('/_next/static/'.length) + ), + 'utf8' + ) + ) + return res.end() + } else { + const re = require(`./.next/serverless/pages${pathname}`) + return typeof re.render === 'function' + ? re.render(req, res) + : re.default(req, res) + } +}) + +server.listen(process.env.PORT, () => { + console.log('ready on', process.env.PORT) +}) diff --git a/test/integration/getserversideprops-preview/test/index.test.js b/test/integration/getserversideprops-preview/test/index.test.js new file mode 100644 index 000000000000..4bcf4bedf8bc --- /dev/null +++ b/test/integration/getserversideprops-preview/test/index.test.js @@ -0,0 +1,300 @@ +/* eslint-env jest */ +/* global jasmine */ +import cheerio from 'cheerio' +import cookie from 'cookie' +import fs from 'fs-extra' +import { + fetchViaHTTP, + findPort, + initNextServerScript, + killApp, + launchApp, + nextBuild, + nextStart, + renderViaHTTP, +} from 'next-test-utils' +import webdriver from 'next-webdriver' +import os from 'os' +import { join } from 'path' +import qs from 'querystring' + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2 +const appDir = join(__dirname, '..') +const nextConfigPath = join(appDir, 'next.config.js') + +async function getBuildId() { + return fs.readFile(join(appDir, '.next', 'BUILD_ID'), 'utf8') +} + +function getData(html) { + const $ = cheerio.load(html) + const nextData = $('#__NEXT_DATA__') + const preEl = $('#props-pre') + return { nextData: JSON.parse(nextData.html()), pre: preEl.text() } +} + +function runTests(startServer = nextStart) { + it('should compile successfully', async () => { + await fs.remove(join(appDir, '.next')) + const { code, stdout } = await nextBuild(appDir, [], { + stdout: true, + }) + expect(code).toBe(0) + expect(stdout).toMatch(/Compiled successfully/) + }) + + let appPort, app + it('should start production application', async () => { + appPort = await findPort() + app = await startServer(appDir, appPort) + }) + + it('should return page on first request', async () => { + const html = await renderViaHTTP(appPort, '/') + const { nextData, pre } = getData(html) + expect(nextData).toMatchObject({ isFallback: false }) + expect(pre).toBe('undefined and undefined') + }) + + it('should return page on second request', async () => { + const html = await renderViaHTTP(appPort, '/') + const { nextData, pre } = getData(html) + expect(nextData).toMatchObject({ isFallback: false }) + expect(pre).toBe('undefined and undefined') + }) + + let previewCookieString + it('should enable preview mode', async () => { + const res = await fetchViaHTTP(appPort, '/api/preview', { lets: 'goooo' }) + expect(res.status).toBe(200) + + const cookies = res.headers + .get('set-cookie') + .split(',') + .map(cookie.parse) + + expect(cookies.length).toBe(2) + expect(cookies[0]).toMatchObject({ Path: '/', SameSite: 'Strict' }) + expect(cookies[0]).toHaveProperty('__prerender_bypass') + expect(cookies[0]).not.toHaveProperty('Max-Age') + expect(cookies[1]).toMatchObject({ Path: '/', SameSite: 'Strict' }) + expect(cookies[1]).toHaveProperty('__next_preview_data') + expect(cookies[1]).not.toHaveProperty('Max-Age') + + previewCookieString = + cookie.serialize('__prerender_bypass', cookies[0].__prerender_bypass) + + '; ' + + cookie.serialize('__next_preview_data', cookies[1].__next_preview_data) + }) + + it('should not return fallback page on preview request', async () => { + const res = await fetchViaHTTP( + appPort, + '/', + {}, + { headers: { Cookie: previewCookieString } } + ) + const html = await res.text() + + const { nextData, pre } = getData(html) + expect(res.headers.get('cache-control')).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + expect(nextData).toMatchObject({ isFallback: false }) + expect(pre).toBe('true and {"lets":"goooo"}') + }) + + it('should return correct caching headers for data preview request', async () => { + const res = await fetchViaHTTP( + appPort, + `/_next/data/${encodeURI(await getBuildId())}/index.json`, + {}, + { headers: { Cookie: previewCookieString } } + ) + const json = await res.json() + + expect(res.headers.get('cache-control')).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + expect(json).toMatchObject({ + pageProps: { + preview: true, + previewData: { lets: 'goooo' }, + }, + }) + }) + + it('should return cookies to be expired on reset request', async () => { + const res = await fetchViaHTTP( + appPort, + '/api/reset', + {}, + { headers: { Cookie: previewCookieString } } + ) + expect(res.status).toBe(200) + + const cookies = res.headers + .get('set-cookie') + .replace(/(=\w{3}),/g, '$1') + .split(',') + .map(cookie.parse) + + expect(cookies.length).toBe(2) + expect(cookies[0]).toMatchObject({ + Path: '/', + SameSite: 'Strict', + Expires: 'Thu 01 Jan 1970 00:00:00 GMT', + }) + expect(cookies[0]).toHaveProperty('__prerender_bypass') + expect(cookies[0]).not.toHaveProperty('Max-Age') + expect(cookies[1]).toMatchObject({ + Path: '/', + SameSite: 'Strict', + Expires: 'Thu 01 Jan 1970 00:00:00 GMT', + }) + expect(cookies[1]).toHaveProperty('__next_preview_data') + expect(cookies[1]).not.toHaveProperty('Max-Age') + }) + + /** @type import('next-webdriver').Chain */ + let browser + it('should start the client-side browser', async () => { + browser = await webdriver( + appPort, + '/api/preview?' + qs.stringify({ client: 'mode' }) + ) + }) + + it('should fetch preview data', async () => { + await browser.get(`http://localhost:${appPort}/`) + await browser.waitForElementByCss('#props-pre') + // expect(await browser.elementById('props-pre').text()).toBe('Has No Props') + // await new Promise(resolve => setTimeout(resolve, 2000)) + expect(await browser.elementById('props-pre').text()).toBe( + 'true and {"client":"mode"}' + ) + }) + + it('should fetch prerendered data', async () => { + await browser.get(`http://localhost:${appPort}/api/reset`) + + await browser.get(`http://localhost:${appPort}/`) + await browser.waitForElementByCss('#props-pre') + expect(await browser.elementById('props-pre').text()).toBe( + 'undefined and undefined' + ) + }) + + afterAll(async () => { + await browser.close() + await killApp(app) + }) +} + +const startServerlessEmulator = async (dir, port) => { + const scriptPath = join(dir, 'server.js') + const env = Object.assign( + {}, + { ...process.env }, + { PORT: port, BUILD_ID: await getBuildId() } + ) + return initNextServerScript(scriptPath, /ready on/i, env) +} + +describe('Prerender Preview Mode', () => { + describe('Development Mode', () => { + beforeAll(async () => { + await fs.remove(nextConfigPath) + }) + + let appPort, app + it('should start development application', async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + + let previewCookieString + it('should enable preview mode', async () => { + const res = await fetchViaHTTP(appPort, '/api/preview', { lets: 'goooo' }) + expect(res.status).toBe(200) + + const cookies = res.headers + .get('set-cookie') + .split(',') + .map(cookie.parse) + + expect(cookies.length).toBe(2) + previewCookieString = + cookie.serialize('__prerender_bypass', cookies[0].__prerender_bypass) + + '; ' + + cookie.serialize('__next_preview_data', cookies[1].__next_preview_data) + }) + + it('should return cookies to be expired after dev server reboot', async () => { + await killApp(app) + app = await launchApp(appDir, appPort) + + const res = await fetchViaHTTP( + appPort, + '/', + {}, + { headers: { Cookie: previewCookieString } } + ) + expect(res.status).toBe(200) + + const body = await res.text() + // "err":{"name":"TypeError","message":"Cannot read property 'previewModeId' of undefined" + expect(body).not.toContain('err') + expect(body).not.toContain('TypeError') + expect(body).not.toContain('previewModeId') + + const cookies = res.headers + .get('set-cookie') + .replace(/(=\w{3}),/g, '$1') + .split(',') + .map(cookie.parse) + + expect(cookies.length).toBe(2) + }) + + afterAll(async () => { + await killApp(app) + }) + }) + + describe('Server Mode', () => { + beforeAll(async () => { + await fs.remove(nextConfigPath) + }) + + runTests() + }) + + describe('Serverless Mode', () => { + beforeAll(async () => { + await fs.writeFile( + nextConfigPath, + `module.exports = { target: 'experimental-serverless-trace' }` + os.EOL + ) + }) + afterAll(async () => { + await fs.remove(nextConfigPath) + }) + + runTests() + }) + + describe('Emulated Serverless Mode', () => { + beforeAll(async () => { + await fs.writeFile( + nextConfigPath, + `module.exports = { target: 'experimental-serverless-trace' }` + os.EOL + ) + }) + afterAll(async () => { + await fs.remove(nextConfigPath) + }) + + runTests(startServerlessEmulator) + }) +})