From 440d855e927014361f91849cf95362d578ea3016 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 13 Jan 2020 16:12:01 -0600 Subject: [PATCH 01/23] Add support for unstable_getServerProps --- .../build/babel/plugins/next-ssg-transform.ts | 2 + packages/next/build/index.ts | 57 +++- packages/next/build/utils.ts | 19 +- .../webpack/loaders/next-serverless-loader.ts | 23 +- packages/next/export/index.ts | 6 +- packages/next/export/worker.js | 8 +- packages/next/lib/constants.ts | 4 + .../next-server/server/load-components.ts | 8 + .../next/next-server/server/next-server.ts | 77 +++-- packages/next/next-server/server/render.tsx | 45 ++- .../getserverprops/pages/another/index.js | 36 ++ .../pages/blog/[post]/[comment].js | 24 ++ .../getserverprops/pages/blog/[post]/index.js | 36 ++ .../getserverprops/pages/blog/index.js | 22 ++ .../pages/default-revalidate.js | 23 ++ .../integration/getserverprops/pages/index.js | 42 +++ .../getserverprops/pages/normal.js | 1 + .../getserverprops/pages/something.js | 32 ++ .../pages/user/[user]/profile.js | 22 ++ .../getserverprops/test/index.test.js | 308 ++++++++++++++++++ test/integration/getserverprops/world.txt | 1 + 21 files changed, 732 insertions(+), 64 deletions(-) create mode 100644 test/integration/getserverprops/pages/another/index.js create mode 100644 test/integration/getserverprops/pages/blog/[post]/[comment].js create mode 100644 test/integration/getserverprops/pages/blog/[post]/index.js create mode 100644 test/integration/getserverprops/pages/blog/index.js create mode 100644 test/integration/getserverprops/pages/default-revalidate.js create mode 100644 test/integration/getserverprops/pages/index.js create mode 100644 test/integration/getserverprops/pages/normal.js create mode 100644 test/integration/getserverprops/pages/something.js create mode 100644 test/integration/getserverprops/pages/user/[user]/profile.js create mode 100644 test/integration/getserverprops/test/index.test.js create mode 100644 test/integration/getserverprops/world.txt diff --git a/packages/next/build/babel/plugins/next-ssg-transform.ts b/packages/next/build/babel/plugins/next-ssg-transform.ts index d9230fd16d9b243..5219cfa43d9f2a2 100644 --- a/packages/next/build/babel/plugins/next-ssg-transform.ts +++ b/packages/next/build/babel/plugins/next-ssg-transform.ts @@ -6,10 +6,12 @@ const prerenderId = '__NEXT_SPR' export const EXPORT_NAME_GET_STATIC_PROPS = 'unstable_getStaticProps' export const EXPORT_NAME_GET_STATIC_PATHS = 'unstable_getStaticPaths' +export const EXPORT_NAME_GET_SERVER_PROPS = 'unstable_getServerProps' const ssgExports = new Set([ EXPORT_NAME_GET_STATIC_PROPS, EXPORT_NAME_GET_STATIC_PATHS, + EXPORT_NAME_GET_SERVER_PROPS, ]) type PluginState = { diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 173ee869bcd5064..2df90e754835a15 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -254,22 +254,23 @@ export default async function build(dir: string, conf = null): Promise { } } + const routesManifestPath = path.join(distDir, ROUTES_MANIFEST) + const routesManifest: any = { + version: 1, + basePath: config.experimental.basePath, + redirects: redirects.map(r => buildCustomRoute(r, 'redirect')), + rewrites: rewrites.map(r => buildCustomRoute(r, 'rewrite')), + headers: headers.map(r => buildCustomRoute(r, 'header')), + dynamicRoutes: getSortedRoutes(dynamicRoutes).map(page => ({ + page, + regex: getRouteRegex(page).re.source, + })), + } + await mkdirp(distDir) - await fsWriteFile( - path.join(distDir, ROUTES_MANIFEST), - JSON.stringify({ - version: 1, - basePath: config.experimental.basePath, - redirects: redirects.map(r => buildCustomRoute(r, 'redirect')), - rewrites: rewrites.map(r => buildCustomRoute(r, 'rewrite')), - headers: headers.map(r => buildCustomRoute(r, 'header')), - dynamicRoutes: getSortedRoutes(dynamicRoutes).map(page => ({ - page, - regex: getRouteRegex(page).re.source, - })), - }), - 'utf8' - ) + // We need to write the manifest with rewrites before build + // so serverless can import the manifest + await fsWriteFile(routesManifestPath, JSON.stringify(routesManifest), 'utf8') const configs = await Promise.all([ getBaseWebpackConfig(dir, { @@ -401,6 +402,7 @@ export default async function build(dir: string, conf = null): Promise { const staticPages = new Set() const invalidPages = new Set() const hybridAmpPages = new Set() + const serverPropsPages = new Set() const additionalSprPaths = new Map>() const pageInfos = new Map() const pagesManifest = JSON.parse(await fsReadFile(manifestPath, 'utf8')) @@ -500,6 +502,8 @@ export default async function build(dir: string, conf = null): Promise { } else if (result.static && customAppGetInitialProps === false) { staticPages.add(page) isStatic = true + } else if (result.serverProps) { + serverPropsPages.add(page) } } catch (err) { if (err.message !== 'INVALID_DEFAULT_EXPORT') throw err @@ -520,6 +524,29 @@ export default async function build(dir: string, conf = null): Promise { ) staticCheckWorkers.end() + if (serverPropsPages.size > 0) { + // We update the routes manifest after the build with the + // serverProps routes since we can't determine this until after build + routesManifest.serverPropsRoutes = {} + + for (const page of serverPropsPages) { + routesManifest.serverPropsRoutes[page] = { + page, + dataRoute: path.posix.join( + '/_next/data', + buildId, + `${page === '/' ? '/index' : page}.json` + ), + } + } + + await fsWriteFile( + routesManifestPath, + JSON.stringify(routesManifest), + 'utf8' + ) + } + if (invalidPages.size > 0) { throw new Error( `Build optimization failed: found page${ diff --git a/packages/next/build/utils.ts b/packages/next/build/utils.ts index 7dcc9c7f7d3b893..eaa6523268d24f6 100644 --- a/packages/next/build/utils.ts +++ b/packages/next/build/utils.ts @@ -5,7 +5,11 @@ import path from 'path' import { isValidElementType } from 'react-is' import stripAnsi from 'strip-ansi' import { Redirect, Rewrite } from '../lib/check-custom-routes' -import { SPR_GET_INITIAL_PROPS_CONFLICT } from '../lib/constants' +import { + SPR_GET_INITIAL_PROPS_CONFLICT, + SERVER_PROPS_GET_INIT_PROPS_CONFLICT, + SERVER_PROPS_SPR_CONFLICT, +} from '../lib/constants' import prettyBytes from '../lib/pretty-bytes' import { recursiveReadDir } from '../lib/recursive-readdir' import { DEFAULT_REDIRECT_STATUS } from '../next-server/lib/constants' @@ -482,6 +486,7 @@ export async function isPageStatic( static?: boolean prerender?: boolean isHybridAmp?: boolean + serverProps?: boolean prerenderRoutes?: string[] | undefined }> { try { @@ -496,6 +501,7 @@ export async function isPageStatic( const hasGetInitialProps = !!(Comp as any).getInitialProps const hasStaticProps = !!mod.unstable_getStaticProps const hasStaticPaths = !!mod.unstable_getStaticPaths + const hasServerProps = !!mod.unstable_getServerProps const hasLegacyStaticParams = !!mod.unstable_getStaticParams if (hasLegacyStaticParams) { @@ -510,6 +516,14 @@ export async function isPageStatic( throw new Error(SPR_GET_INITIAL_PROPS_CONFLICT) } + if (hasGetInitialProps && hasServerProps) { + throw new Error(SERVER_PROPS_GET_INIT_PROPS_CONFLICT) + } + + if (hasStaticProps && hasServerProps) { + throw new Error(SERVER_PROPS_SPR_CONFLICT) + } + // A page cannot have static parameters if it is not a dynamic page. if (hasStaticProps && hasStaticPaths && !isDynamicRoute(page)) { throw new Error( @@ -579,9 +593,10 @@ export async function isPageStatic( const config = mod.config || {} return { - static: !hasStaticProps && !hasGetInitialProps, + static: !hasStaticProps && !hasGetInitialProps && !hasServerProps, isHybridAmp: config.amp === 'hybrid', prerenderRoutes: prerenderPaths, + serverProps: hasServerProps, prerender: hasStaticProps, } } catch (err) { diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index fdc47e8fde563ee..596d2e6924bf0dc 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -185,6 +185,7 @@ const nextServerlessLoader: loader.Loader = function() { export const unstable_getStaticProps = ComponentInfo['unstable_getStaticProp' + 's'] export const unstable_getStaticParams = ComponentInfo['unstable_getStaticParam' + 's'] export const unstable_getStaticPaths = ComponentInfo['unstable_getStaticPath' + 's'] + export const unstable_getServerProps = ComponentInfo['unstable_getServerProp' + 's'] ${dynamicRouteMatcher} ${handleRewrites} @@ -206,6 +207,7 @@ const nextServerlessLoader: loader.Loader = function() { Document, buildManifest, unstable_getStaticProps, + unstable_getServerProps, unstable_getStaticPaths, reactLoadableManifest, canonicalBase: "${canonicalBase}", @@ -213,10 +215,10 @@ const nextServerlessLoader: loader.Loader = function() { assetPrefix: "${assetPrefix}", ..._renderOpts } - let sprData = false + let _nextData = false if (req.url.match(/_next\\/data/)) { - sprData = true + _nextData = true req.url = req.url .replace(new RegExp('/_next/data/${escapedBuildId}/'), '/') .replace(/\\.json$/, '') @@ -235,7 +237,7 @@ const nextServerlessLoader: loader.Loader = function() { ${page === '/_error' ? `res.statusCode = 404` : ''} ${ pageIsDynamicRoute - ? `const params = fromExport && !unstable_getStaticProps ? {} : dynamicRouteMatcher(parsedUrl.pathname) || {};` + ? `const params = fromExport && !unstable_getStaticProps && !unstable_getServerProps ? {} : dynamicRouteMatcher(parsedUrl.pathname) || {};` : `const params = {};` } ${ @@ -273,14 +275,17 @@ const nextServerlessLoader: loader.Loader = function() { } let result = await renderToHTML(req, res, "${page}", Object.assign({}, unstable_getStaticProps ? {} : parsedUrl.query, nowParams ? nowParams : params, _params), renderOpts) - if (sprData && !fromExport) { - const payload = JSON.stringify(renderOpts.sprData) + if (_nextData && !fromExport) { + const payload = JSON.stringify(renderOpts.pageData) res.setHeader('Content-Type', 'application/json') res.setHeader('Content-Length', Buffer.byteLength(payload)) - res.setHeader( - 'Cache-Control', - \`s-maxage=\${renderOpts.revalidate}, stale-while-revalidate\` - ) + + if (renderOpts.revalidate) { + res.setHeader( + 'Cache-Control', + \`s-maxage=\${renderOpts.revalidate}, stale-while-revalidate\` + ) + } res.end(payload) return null } diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index da26b61dd831637..42371a80d6466a7 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -276,7 +276,7 @@ export default async function( } const progress = !options.silent && createProgress(filteredPaths.length) - const sprDataDir = options.buildExport + const pageDataDir = options.buildExport ? outDir : join(outDir, '_next/data', buildId) @@ -318,7 +318,7 @@ export default async function( distDir, buildId, outDir, - sprDataDir, + pageDataDir, renderOpts, serverRuntimeConfig, subFolders, @@ -360,7 +360,7 @@ export default async function( subFolders && route !== '/index' ? `${sep}index` : '' }.html` ) - const jsonDest = join(sprDataDir, `${route}.json`) + const jsonDest = join(pageDataDir, `${route}.json`) await mkdirp(dirname(htmlDest)) await mkdirp(dirname(jsonDest)) diff --git a/packages/next/export/worker.js b/packages/next/export/worker.js index 072faae8dd11667..27f0dd8d4b0defc 100644 --- a/packages/next/export/worker.js +++ b/packages/next/export/worker.js @@ -25,7 +25,7 @@ export default async function({ distDir, buildId, outDir, - sprDataDir, + pageDataDir, renderOpts, buildExport, serverRuntimeConfig, @@ -234,14 +234,14 @@ export default async function({ } } - if (curRenderOpts.sprData) { + if (curRenderOpts.pageData) { const dataFile = join( - sprDataDir, + pageDataDir, htmlFilename.replace(/\.html$/, '.json') ) await mkdirp(dirname(dataFile)) - await writeFileP(dataFile, JSON.stringify(curRenderOpts.sprData), 'utf8') + await writeFileP(dataFile, JSON.stringify(curRenderOpts.pageData), 'utf8') } results.fromBuildExportRevalidate = curRenderOpts.revalidate diff --git a/packages/next/lib/constants.ts b/packages/next/lib/constants.ts index be1c47741295a69..04c38ec8af2c324 100644 --- a/packages/next/lib/constants.ts +++ b/packages/next/lib/constants.ts @@ -25,3 +25,7 @@ export const DOT_NEXT_ALIAS = 'private-dot-next' export const PUBLIC_DIR_MIDDLEWARE_CONFLICT = `You can not have a '_next' folder inside of your public folder. This conflicts with the internal '/_next' route. https://err.sh/zeit/next.js/public-next-folder-conflict` export const SPR_GET_INITIAL_PROPS_CONFLICT = `You can not use getInitialProps with unstable_getStaticProps. To use SPR, please remove your getInitialProps` + +export const SERVER_PROPS_GET_INIT_PROPS_CONFLICT = `You can not use getInitialProps with unstable_getServerProps. Please remove one or the other` + +export const SERVER_PROPS_SPR_CONFLICT = `You can not use unstable_getStaticProps with unstable_getServerProps. To use SPR, please remove your unstable_getServerProps` diff --git a/packages/next/next-server/server/load-components.ts b/packages/next/next-server/server/load-components.ts index a6aee255a0cc045..59ade5681b33373 100644 --- a/packages/next/next-server/server/load-components.ts +++ b/packages/next/next-server/server/load-components.ts @@ -1,3 +1,5 @@ +import { IncomingMessage, ServerResponse } from 'http' +import { ParsedUrlQuery } from 'querystring' import { BUILD_MANIFEST, CLIENT_STATIC_FILES_PATH, @@ -22,6 +24,11 @@ export type LoadComponentsReturnType = { revalidate?: number | boolean } unstable_getStaticPaths?: () => void + unstable_getServerProps?: (context: { + req: IncomingMessage + res: ServerResponse + query: ParsedUrlQuery + }) => Promise<{ [key: string]: any }> buildManifest?: any reactLoadableManifest?: any Document?: any @@ -90,6 +97,7 @@ export async function loadComponents( DocumentMiddleware, reactLoadableManifest, pageConfig: ComponentMod.config || {}, + unstable_getServerProps: ComponentMod.unstable_getServerProps, unstable_getStaticProps: ComponentMod.unstable_getStaticProps, unstable_getStaticPaths: ComponentMod.unstable_getStaticPaths, } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ddb5d861aeecd00..daae97544e4750a 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -380,7 +380,7 @@ export default class Server { req, res, pathname, - { _nextSprData: '1' }, + { _nextDataReq: '1' }, parsedUrl ) return { @@ -831,10 +831,36 @@ export default class Server { typeof result.Component.renderReqToHTML === 'function' const isSpr = !!result.unstable_getStaticProps + // Toggle whether or not this is a Data request + const isDataReq = query._nextDataReq + delete query._nextDataReq + + // Serverless requests need its URL transformed back into the original + // request path (to emulate lambda behavior in production) + if (isLikeServerless && isDataReq) { + let { pathname } = parseUrl(req.url || '', true) + pathname = !pathname || pathname === '/' ? '/index' : pathname + req.url = `/_next/data/${this.buildId}${pathname}.json` + } + // non-spr requests should render like normal if (!isSpr) { // handle serverless if (isLikeServerless) { + if (isDataReq) { + const renderResult = await result.Component.renderReqToHTML( + req, + res, + true + ) + + this.__sendPayload( + res, + JSON.stringify(renderResult?.renderOpts?.pageData), + 'application/json' + ) + return null + } const curUrl = parseUrl(req.url!, true) req.url = formatUrl({ ...curUrl, @@ -847,30 +873,37 @@ export default class Server { return result.Component.renderReqToHTML(req, res) } + if (isDataReq && typeof result.unstable_getServerProps === 'function') { + const props = await renderToHTML(req, res, pathname, query, { + ...result, + ...opts, + isDataReq, + }) + this.__sendPayload(res, JSON.stringify(props), 'application/json') + return null + } + return renderToHTML(req, res, pathname, query, { ...result, ...opts, }) } - // Toggle whether or not this is an SPR Data request - const isSprData = isSpr && query._nextSprData - delete query._nextSprData - // Compute the SPR cache key const sprCacheKey = parseUrl(req.url || '').pathname! + const isPageData = isSpr && isDataReq // Complete the response with cached data if its present const cachedData = await getSprCache(sprCacheKey) if (cachedData) { - const data = isSprData + const data = isPageData ? JSON.stringify(cachedData.pageData) : cachedData.html this.__sendPayload( res, data, - isSprData ? 'application/json' : 'text/html; charset=utf-8', + isPageData ? 'application/json' : 'text/html; charset=utf-8', cachedData.curRevalidate ) @@ -882,20 +915,12 @@ export default class Server { // If we're here, that means data is missing or it's stale. - // Serverless requests need its URL transformed back into the original - // request path (to emulate lambda behavior in production) - if (isLikeServerless && isSprData) { - let { pathname } = parseUrl(req.url || '', true) - pathname = !pathname || pathname === '/' ? '/index' : pathname - req.url = `/_next/data/${this.buildId}${pathname}.json` - } - const doRender = withCoalescedInvoke(async function(): Promise<{ html: string | null - sprData: any + pageData: any sprRevalidate: number | false }> { - let sprData: any + let pageData: any let html: string | null let sprRevalidate: number | false @@ -905,7 +930,7 @@ export default class Server { renderResult = await result.Component.renderReqToHTML(req, res, true) html = renderResult.html - sprData = renderResult.renderOpts.sprData + pageData = renderResult.renderOpts.pageData sprRevalidate = renderResult.renderOpts.revalidate } else { const renderOpts = { @@ -915,21 +940,21 @@ export default class Server { renderResult = await renderToHTML(req, res, pathname, query, renderOpts) html = renderResult - sprData = renderOpts.sprData + pageData = renderOpts.pageData sprRevalidate = renderOpts.revalidate } - return { html, sprData, sprRevalidate } + return { html, pageData, sprRevalidate } }) return doRender(sprCacheKey, []).then( - async ({ isOrigin, value: { html, sprData, sprRevalidate } }) => { + async ({ isOrigin, value: { html, pageData, sprRevalidate } }) => { // Respond to the request if a payload wasn't sent above (from cache) if (!isResSent(res)) { this.__sendPayload( res, - isSprData ? JSON.stringify(sprData) : html, - isSprData ? 'application/json' : 'text/html; charset=utf-8', + isPageData ? JSON.stringify(pageData) : html, + isPageData ? 'application/json' : 'text/html; charset=utf-8', sprRevalidate ) } @@ -938,7 +963,7 @@ export default class Server { if (isOrigin) { await setSprCache( sprCacheKey, - { html: html!, pageData: sprData }, + { html: html!, pageData: pageData }, sprRevalidate ) } @@ -969,7 +994,7 @@ export default class Server { res, pathname, result.unstable_getStaticProps - ? { _nextSprData: query._nextSprData } + ? { _nextDataReq: query._nextDataReq } : query, result, { ...this.renderOpts, amphtml, hasAmp } @@ -995,7 +1020,7 @@ export default class Server { // only add params for SPR enabled pages { ...(result.unstable_getStaticProps - ? { _nextSprData: query._nextSprData } + ? { _nextDataReq: query._nextDataReq } : query), ...params, }, diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index aebb107c555c153..6147ce3af21b024 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -28,7 +28,11 @@ import { isInAmpMode } from '../lib/amp' // Uses a module path because of the compiled output directory location import { PageConfig } from 'next/types' import { isDynamicRoute } from '../lib/router/utils/is-dynamic' -import { SPR_GET_INITIAL_PROPS_CONFLICT } from '../../lib/constants' +import { + SPR_GET_INITIAL_PROPS_CONFLICT, + SERVER_PROPS_GET_INIT_PROPS_CONFLICT, + SERVER_PROPS_SPR_CONFLICT, +} from '../../lib/constants' import { AMP_RENDER_TARGET } from '../lib/constants' export type ManifestItem = { @@ -150,11 +154,17 @@ type RenderOpts = { ampValidator?: (html: string, pathname: string) => Promise unstable_getStaticProps?: (params: { params: any | undefined - }) => { + }) => Promise<{ props: any revalidate?: number | boolean - } + }> unstable_getStaticPaths?: () => void + unstable_getServerProps?: (context: { + req: IncomingMessage + res: ServerResponse + query: ParsedUrlQuery + }) => Promise<{ [key: string]: any }> + isDataReq: boolean } function renderDocument( @@ -272,6 +282,8 @@ export async function renderToHTML( ErrorDebug, unstable_getStaticProps, unstable_getStaticPaths, + unstable_getServerProps, + isDataReq, } = renderOpts const callMiddleware = async (method: string, args: any[], props = false) => { @@ -307,7 +319,10 @@ export async function renderToHTML( const hasPageGetInitialProps = !!(Component as any).getInitialProps const isAutoExport = - !hasPageGetInitialProps && defaultAppGetInitialProps && !isSpr + !hasPageGetInitialProps && + defaultAppGetInitialProps && + !isSpr && + !unstable_getServerProps if ( process.env.NODE_ENV !== 'production' && @@ -327,6 +342,14 @@ export async function renderToHTML( throw new Error(SPR_GET_INITIAL_PROPS_CONFLICT + ` ${pathname}`) } + if (hasPageGetInitialProps && unstable_getServerProps) { + throw new Error(SERVER_PROPS_GET_INIT_PROPS_CONFLICT + ` ${pathname}`) + } + + if (unstable_getServerProps && isSpr) { + throw new Error(SERVER_PROPS_SPR_CONFLICT + ` ${pathname}`) + } + if (!!unstable_getStaticPaths && !isSpr) { throw new Error( `unstable_getStaticPaths was added without a unstable_getStaticProps in ${pathname}. Without unstable_getStaticProps, unstable_getStaticPaths does nothing` @@ -470,7 +493,7 @@ export async function renderToHTML( props.pageProps = data.props // pass up revalidate and props for export ;(renderOpts as any).revalidate = data.revalidate - ;(renderOpts as any).sprData = props + ;(renderOpts as any).pageData = props } } catch (err) { if (!dev || !err) throw err @@ -478,6 +501,18 @@ export async function renderToHTML( renderOpts.err = err } + if (unstable_getServerProps) { + props.pageProps = await unstable_getServerProps({ + query, + req, + res, + }) + ;(renderOpts as any).pageData = props + } + // We only need to do this if we want to support calling + // _app's getInitialProps for getServerProps if not this can be removed + if (isDataReq) return props + // the response might be finished on the getInitialProps call if (isResSent(res) && !isSpr) return null diff --git a/test/integration/getserverprops/pages/another/index.js b/test/integration/getserverprops/pages/another/index.js new file mode 100644 index 000000000000000..7f588dcf3a876fe --- /dev/null +++ b/test/integration/getserverprops/pages/another/index.js @@ -0,0 +1,36 @@ +import Link from 'next/link' +import fs from 'fs' +import findUp from 'find-up' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps() { + const text = fs + .readFileSync( + findUp.sync('world.txt', { + // prevent webpack from intercepting + // eslint-disable-next-line no-eval + cwd: eval(`__dirname`), + }), + 'utf8' + ) + .trim() + + return { + world: text, + time: new Date().getTime(), + } +} + +export default ({ world, time }) => ( + <> +

hello {world}

+ time: {time} + + to home + +
+ + to something + + +) diff --git a/test/integration/getserverprops/pages/blog/[post]/[comment].js b/test/integration/getserverprops/pages/blog/[post]/[comment].js new file mode 100644 index 000000000000000..840676c6f756b70 --- /dev/null +++ b/test/integration/getserverprops/pages/blog/[post]/[comment].js @@ -0,0 +1,24 @@ +import React from 'react' +import Link from 'next/link' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps({ query }) { + return { + post: query.post, + comment: query.comment, + time: new Date().getTime(), + } +} + +export default ({ post, comment, time }) => { + return ( + <> +

Post: {post}

+

Comment: {comment}

+ time: {time} + + to home + + + ) +} diff --git a/test/integration/getserverprops/pages/blog/[post]/index.js b/test/integration/getserverprops/pages/blog/[post]/index.js new file mode 100644 index 000000000000000..155a19d77f4e605 --- /dev/null +++ b/test/integration/getserverprops/pages/blog/[post]/index.js @@ -0,0 +1,36 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps({ query }) { + if (query.post === 'post-10') { + await new Promise(resolve => { + setTimeout(() => resolve(), 1000) + }) + } + + if (query.post === 'post-100') { + throw new Error('such broken..') + } + + return { + query, + post: query.post, + time: (await import('perf_hooks')).performance.now(), + } +} + +export default ({ post, time, query }) => { + return ( + <> +

Post: {post}

+ time: {time} +
{JSON.stringify(query)}
+
{JSON.stringify(useRouter().query)}
+ + to home + + + ) +} diff --git a/test/integration/getserverprops/pages/blog/index.js b/test/integration/getserverprops/pages/blog/index.js new file mode 100644 index 000000000000000..341930d994fc23b --- /dev/null +++ b/test/integration/getserverprops/pages/blog/index.js @@ -0,0 +1,22 @@ +import React from 'react' +import Link from 'next/link' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps() { + return { + slugs: ['post-1', 'post-2'], + time: (await import('perf_hooks')).performance.now(), + } +} + +export default ({ slugs, time }) => { + return ( + <> +

Posts: {JSON.stringify(slugs)}

+ time: {time} + + to home + + + ) +} diff --git a/test/integration/getserverprops/pages/default-revalidate.js b/test/integration/getserverprops/pages/default-revalidate.js new file mode 100644 index 000000000000000..d078e6a08e8a875 --- /dev/null +++ b/test/integration/getserverprops/pages/default-revalidate.js @@ -0,0 +1,23 @@ +import Link from 'next/link' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps() { + return { + world: 'world', + time: new Date().getTime(), + } +} + +export default ({ world, time }) => ( + <> +

hello {world}

+ time: {time} + + to home + +
+ + to something + + +) diff --git a/test/integration/getserverprops/pages/index.js b/test/integration/getserverprops/pages/index.js new file mode 100644 index 000000000000000..f41bb023011461f --- /dev/null +++ b/test/integration/getserverprops/pages/index.js @@ -0,0 +1,42 @@ +import Link from 'next/link' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps() { + return { + world: 'world', + time: new Date().getTime(), + } +} + +const Page = ({ world, time }) => { + return ( + <> +

hello {world}

+ time: {time} + + to another + +
+ + to something + +
+ + to normal + +
+ + to dynamic + + + to broken + +
+ + to another dynamic + + + ) +} + +export default Page diff --git a/test/integration/getserverprops/pages/normal.js b/test/integration/getserverprops/pages/normal.js new file mode 100644 index 000000000000000..75ad8dfee1722d9 --- /dev/null +++ b/test/integration/getserverprops/pages/normal.js @@ -0,0 +1 @@ +export default () =>

a normal page

diff --git a/test/integration/getserverprops/pages/something.js b/test/integration/getserverprops/pages/something.js new file mode 100644 index 000000000000000..94965ee53975c75 --- /dev/null +++ b/test/integration/getserverprops/pages/something.js @@ -0,0 +1,32 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps({ query }) { + return { + world: 'world', + params: query || {}, + time: new Date().getTime(), + random: Math.random(), + } +} + +export default ({ world, time, params, random }) => { + return ( + <> +

hello: {world}

+ time: {time} +
{random}
+
{JSON.stringify(params)}
+
{JSON.stringify(useRouter().query)}
+ + to home + +
+ + to another + + + ) +} diff --git a/test/integration/getserverprops/pages/user/[user]/profile.js b/test/integration/getserverprops/pages/user/[user]/profile.js new file mode 100644 index 000000000000000..722fbc24671e388 --- /dev/null +++ b/test/integration/getserverprops/pages/user/[user]/profile.js @@ -0,0 +1,22 @@ +import React from 'react' +import Link from 'next/link' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps({ query }) { + return { + user: query.user, + time: (await import('perf_hooks')).performance.now(), + } +} + +export default ({ user, time }) => { + return ( + <> +

User: {user}

+ time: {time} + + to home + + + ) +} diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js new file mode 100644 index 000000000000000..f9d2ad05303bdfb --- /dev/null +++ b/test/integration/getserverprops/test/index.test.js @@ -0,0 +1,308 @@ +/* eslint-env jest */ +/* global jasmine */ +import fs from 'fs-extra' +import { join } from 'path' +import webdriver from 'next-webdriver' +import cheerio from 'cheerio' +import { + renderViaHTTP, + fetchViaHTTP, + findPort, + launchApp, + killApp, + waitFor, + nextBuild, + nextStart, +} from 'next-test-utils' + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2 +const appDir = join(__dirname, '..') +const nextConfig = join(appDir, 'next.config.js') +let app +let appPort +let buildId + +const expectedManifestRoutes = () => ({ + '/user/[user]/profile': { + dataRoute: `/_next/data/${buildId}/user/[user]/profile.json`, + page: '/user/[user]/profile', + }, + '/': { + dataRoute: `/_next/data/${buildId}/index.json`, + page: '/', + }, + '/blog/[post]/[comment]': { + dataRoute: `/_next/data/${buildId}/blog/[post]/[comment].json`, + page: '/blog/[post]/[comment]', + }, + '/blog': { + dataRoute: `/_next/data/${buildId}/blog.json`, + page: '/blog', + }, + '/default-revalidate': { + dataRoute: `/_next/data/${buildId}/default-revalidate.json`, + page: '/default-revalidate', + }, + '/another': { + dataRoute: `/_next/data/${buildId}/another.json`, + page: '/another', + }, + '/blog/[post]': { + dataRoute: `/_next/data/${buildId}/blog/[post].json`, + page: '/blog/[post]', + }, + '/something': { + dataRoute: `/_next/data/${buildId}/something.json`, + page: '/something', + }, +}) + +const navigateTest = (dev = false) => { + it('should navigate between pages successfully', async () => { + const toBuild = [ + '/', + '/another', + '/something', + '/normal', + '/blog/post-1', + '/blog/post-1/comment-1', + ] + + await Promise.all(toBuild.map(pg => renderViaHTTP(appPort, pg))) + + const browser = await webdriver(appPort, '/') + let text = await browser.elementByCss('p').text() + expect(text).toMatch(/hello.*?world/) + + // hydration + await waitFor(2500) + + // go to /another + async function goFromHomeToAnother() { + await browser.elementByCss('#another').click() + await browser.waitForElementByCss('#home') + text = await browser.elementByCss('p').text() + expect(text).toMatch(/hello.*?world/) + } + await goFromHomeToAnother() + + // go to / + async function goFromAnotherToHome() { + await browser.eval('window.didTransition = 1') + await browser.elementByCss('#home').click() + await browser.waitForElementByCss('#another') + text = await browser.elementByCss('p').text() + expect(text).toMatch(/hello.*?world/) + expect(await browser.eval('window.didTransition')).toBe(1) + } + await goFromAnotherToHome() + + await goFromHomeToAnother() + const snapTime = await browser.elementByCss('#anotherTime').text() + + // Re-visit page + await goFromAnotherToHome() + await goFromHomeToAnother() + + const nextTime = await browser.elementByCss('#anotherTime').text() + if (dev) { + expect(snapTime).not.toMatch(nextTime) + } else { + expect(snapTime).toMatch(nextTime) + } + + // Reset to Home for next test + await goFromAnotherToHome() + + // go to /something + await browser.elementByCss('#something').click() + await browser.waitForElementByCss('#home') + text = await browser.elementByCss('p').text() + expect(text).toMatch(/hello.*?world/) + expect(await browser.eval('window.didTransition')).toBe(1) + + // go to / + await browser.elementByCss('#home').click() + await browser.waitForElementByCss('#post-1') + + // go to /blog/post-1 + await browser.elementByCss('#post-1').click() + await browser.waitForElementByCss('#home') + text = await browser.elementByCss('p').text() + expect(text).toMatch(/Post:.*?post-1/) + expect(await browser.eval('window.didTransition')).toBe(1) + + // go to / + await browser.elementByCss('#home').click() + await browser.waitForElementByCss('#comment-1') + + // go to /blog/post-1/comment-1 + await browser.elementByCss('#comment-1').click() + await browser.waitForElementByCss('#home') + text = await browser.elementByCss('p:nth-child(2)').text() + expect(text).toMatch(/Comment:.*?comment-1/) + expect(await browser.eval('window.didTransition')).toBe(1) + + await browser.close() + }) +} + +const runTests = (dev = false) => { + navigateTest(dev) + + it('should SSR normal page correctly', async () => { + const html = await renderViaHTTP(appPort, '/') + expect(html).toMatch(/hello.*?world/) + }) + + it('should SSR getServerProps page correctly', async () => { + const html = await renderViaHTTP(appPort, '/blog/post-1') + expect(html).toMatch(/Post:.*?post-1/) + }) + + it('should supply query values SSR', async () => { + const html = await renderViaHTTP(appPort, '/blog/post-1?hello=world') + const $ = cheerio.load(html) + const params = $('#params').text() + expect(JSON.parse(params)).toEqual({ hello: 'world', post: 'post-1' }) + const query = $('#query').text() + expect(JSON.parse(query)).toEqual({ hello: 'world', post: 'post-1' }) + }) + + it('should return data correctly', async () => { + const data = JSON.parse( + await renderViaHTTP(appPort, `/_next/data/${buildId}/something.json`) + ) + expect(data.pageProps.world).toBe('world') + }) + + it('should return data correctly for dynamic page', async () => { + const data = JSON.parse( + await renderViaHTTP(appPort, `/_next/data/${buildId}/blog/post-1.json`) + ) + expect(data.pageProps.post).toBe('post-1') + }) + + it('should navigate to a normal page and back', async () => { + const browser = await webdriver(appPort, '/') + let text = await browser.elementByCss('p').text() + expect(text).toMatch(/hello.*?world/) + + await browser.elementByCss('#normal').click() + await browser.waitForElementByCss('#normal-text') + text = await browser.elementByCss('#normal-text').text() + expect(text).toMatch(/a normal page/) + }) + + it('should parse query values on mount correctly', async () => { + const browser = await webdriver(appPort, '/blog/post-1?another=value') + await waitFor(2000) + const text = await browser.elementByCss('#query').text() + expect(text).toMatch(/another.*?value/) + expect(text).toMatch(/post.*?post-1/) + }) + + it('should reload page on failed data request', async () => { + const browser = await webdriver(appPort, '/') + await waitFor(500) + await browser.eval('window.beforeClick = true') + await browser.elementByCss('#broken-post').click() + await waitFor(1000) + expect(await browser.eval('window.beforeClick')).not.toBe('true') + }) + + it('should always call getServerProps without caching', async () => { + const initialRes = await fetchViaHTTP(appPort, '/something') + const initialHtml = await initialRes.text() + expect(initialHtml).toMatch(/hello.*?world/) + + const newRes = await fetchViaHTTP(appPort, '/something') + const newHtml = await newRes.text() + expect(newHtml).toMatch(/hello.*?world/) + expect(initialHtml !== newHtml).toBe(true) + + const newerRes = await fetchViaHTTP(appPort, '/something') + const newerHtml = await newerRes.text() + expect(newerHtml).toMatch(/hello.*?world/) + expect(newHtml !== newerHtml).toBe(true) + }) + + it('should not re-call getServerProps when updating query', async () => { + const browser = await webdriver(appPort, '/something?hello=world') + await waitFor(2000) + + const query = await browser.elementByCss('#query').text() + expect(JSON.parse(query)).toEqual({ hello: 'world' }) + + const { + props: { + pageProps: { random: initialRandom }, + }, + } = await browser.eval('window.__NEXT_DATA__') + + const curRandom = await browser.elementByCss('#random').text() + expect(curRandom).toBe(initialRandom + '') + }) + + if (!dev) { + it('should not fetch data on mount', async () => { + const browser = await webdriver(appPort, '/blog/post-100') + await browser.eval('window.thisShouldStay = true') + await waitFor(2 * 1000) + const val = await browser.eval('window.thisShouldStay') + expect(val).toBe(true) + }) + + it('should output routes-manifest correctly', async () => { + const routesManifest = await fs.readJSON( + join(appDir, '.next/routes-manifest.json') + ) + + expect(routesManifest.serverPropsRoutes).toEqual(expectedManifestRoutes()) + }) + } +} + +describe('unstable_getServerProps', () => { + describe('dev mode', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + buildId = 'development' + }) + afterAll(() => killApp(app)) + + runTests(true) + }) + + describe('serverless mode', () => { + beforeAll(async () => { + await fs.writeFile( + nextConfig, + `module.exports = { target: 'serverless' }`, + 'utf8' + ) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8') + }) + afterAll(() => killApp(app)) + + runTests() + }) + + describe('production mode', () => { + beforeAll(async () => { + await fs.remove(nextConfig) + await nextBuild(appDir, [], { stdout: true }) + + appPort = await findPort() + app = await nextStart(appDir, appPort) + buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8') + }) + afterAll(() => killApp(app)) + + runTests() + }) +}) diff --git a/test/integration/getserverprops/world.txt b/test/integration/getserverprops/world.txt new file mode 100644 index 000000000000000..04fea06420ca608 --- /dev/null +++ b/test/integration/getserverprops/world.txt @@ -0,0 +1 @@ +world \ No newline at end of file From 1ebb040b063edb946939ac135f8c798f7ced6d6e Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 14 Jan 2020 14:44:48 -0600 Subject: [PATCH 02/23] Apply suggestions from review --- packages/next/build/index.ts | 4 ++-- packages/next/build/utils.ts | 12 ++++++------ .../webpack/loaders/next-serverless-loader.ts | 2 ++ packages/next/lib/constants.ts | 4 ++-- .../next/next-server/server/load-components.ts | 1 + packages/next/next-server/server/next-server.ts | 1 + packages/next/next-server/server/render.tsx | 12 ++++++++---- .../getserverprops/pages/blog/[post]/index.js | 14 +++++++------- test/integration/getserverprops/pages/something.js | 4 ++-- test/integration/getserverprops/test/index.test.js | 2 +- 10 files changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 57e88a484f3662b..0c8ed762e8869f5 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -500,11 +500,11 @@ export default async function build(dir: string, conf = null): Promise { additionalSprPaths.set(page, result.prerenderRoutes) ssgPageRoutes = result.prerenderRoutes } + } else if (result.hasServerProps) { + serverPropsPages.add(page) } else if (result.static && customAppGetInitialProps === false) { staticPages.add(page) isStatic = true - } else if (result.serverProps) { - serverPropsPages.add(page) } } catch (err) { if (err.message !== 'INVALID_DEFAULT_EXPORT') throw err diff --git a/packages/next/build/utils.ts b/packages/next/build/utils.ts index a46ad51418e5088..1a5d45f04c0c8ff 100644 --- a/packages/next/build/utils.ts +++ b/packages/next/build/utils.ts @@ -10,9 +10,9 @@ import { getRedirectStatus, } from '../lib/check-custom-routes' import { - SPR_GET_INITIAL_PROPS_CONFLICT, + SSG_GET_INITIAL_PROPS_CONFLICT, SERVER_PROPS_GET_INIT_PROPS_CONFLICT, - SERVER_PROPS_SPR_CONFLICT, + SERVER_PROPS_SSG_CONFLICT, } from '../lib/constants' import prettyBytes from '../lib/pretty-bytes' import { recursiveReadDir } from '../lib/recursive-readdir' @@ -486,7 +486,7 @@ export async function isPageStatic( static?: boolean prerender?: boolean isHybridAmp?: boolean - serverProps?: boolean + hasServerProps?: boolean prerenderRoutes?: string[] | undefined }> { try { @@ -513,7 +513,7 @@ export async function isPageStatic( // A page cannot be prerendered _and_ define a data requirement. That's // contradictory! if (hasGetInitialProps && hasStaticProps) { - throw new Error(SPR_GET_INITIAL_PROPS_CONFLICT) + throw new Error(SSG_GET_INITIAL_PROPS_CONFLICT) } if (hasGetInitialProps && hasServerProps) { @@ -521,7 +521,7 @@ export async function isPageStatic( } if (hasStaticProps && hasServerProps) { - throw new Error(SERVER_PROPS_SPR_CONFLICT) + throw new Error(SERVER_PROPS_SSG_CONFLICT) } // A page cannot have static parameters if it is not a dynamic page. @@ -596,8 +596,8 @@ export async function isPageStatic( static: !hasStaticProps && !hasGetInitialProps && !hasServerProps, isHybridAmp: config.amp === 'hybrid', prerenderRoutes: prerenderPaths, - serverProps: hasServerProps, prerender: hasStaticProps, + hasServerProps, } } catch (err) { if (err.code === 'MODULE_NOT_FOUND') return {} diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 596d2e6924bf0dc..645d8cdec401ab4 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -273,6 +273,8 @@ const nextServerlessLoader: loader.Loader = function() { ` : `const nowParams = null;` } + renderOpts.params = _params || params + let result = await renderToHTML(req, res, "${page}", Object.assign({}, unstable_getStaticProps ? {} : parsedUrl.query, nowParams ? nowParams : params, _params), renderOpts) if (_nextData && !fromExport) { diff --git a/packages/next/lib/constants.ts b/packages/next/lib/constants.ts index 04c38ec8af2c324..1bf62bb57e918f4 100644 --- a/packages/next/lib/constants.ts +++ b/packages/next/lib/constants.ts @@ -24,8 +24,8 @@ export const DOT_NEXT_ALIAS = 'private-dot-next' export const PUBLIC_DIR_MIDDLEWARE_CONFLICT = `You can not have a '_next' folder inside of your public folder. This conflicts with the internal '/_next' route. https://err.sh/zeit/next.js/public-next-folder-conflict` -export const SPR_GET_INITIAL_PROPS_CONFLICT = `You can not use getInitialProps with unstable_getStaticProps. To use SPR, please remove your getInitialProps` +export const SSG_GET_INITIAL_PROPS_CONFLICT = `You can not use getInitialProps with unstable_getStaticProps. To use SSG, please remove your getInitialProps` export const SERVER_PROPS_GET_INIT_PROPS_CONFLICT = `You can not use getInitialProps with unstable_getServerProps. Please remove one or the other` -export const SERVER_PROPS_SPR_CONFLICT = `You can not use unstable_getStaticProps with unstable_getServerProps. To use SPR, please remove your unstable_getServerProps` +export const SERVER_PROPS_SSG_CONFLICT = `You can not use unstable_getStaticProps with unstable_getServerProps. To use SSG, please remove your unstable_getServerProps` diff --git a/packages/next/next-server/server/load-components.ts b/packages/next/next-server/server/load-components.ts index 59ade5681b33373..baa01fb66d28cd5 100644 --- a/packages/next/next-server/server/load-components.ts +++ b/packages/next/next-server/server/load-components.ts @@ -25,6 +25,7 @@ export type LoadComponentsReturnType = { } unstable_getStaticPaths?: () => void unstable_getServerProps?: (context: { + params: { [key: string]: string } req: IncomingMessage res: ServerResponse query: ParsedUrlQuery diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 3683786b6e64d5a..792efc113608130 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1026,6 +1026,7 @@ export default class Server { result, { ...this.renderOpts, + params, amphtml, hasAmp, } diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 6147ce3af21b024..61ee13b683fcd21 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -29,9 +29,9 @@ import { isInAmpMode } from '../lib/amp' import { PageConfig } from 'next/types' import { isDynamicRoute } from '../lib/router/utils/is-dynamic' import { - SPR_GET_INITIAL_PROPS_CONFLICT, + SSG_GET_INITIAL_PROPS_CONFLICT, SERVER_PROPS_GET_INIT_PROPS_CONFLICT, - SERVER_PROPS_SPR_CONFLICT, + SERVER_PROPS_SSG_CONFLICT, } from '../../lib/constants' import { AMP_RENDER_TARGET } from '../lib/constants' @@ -160,11 +160,13 @@ type RenderOpts = { }> unstable_getStaticPaths?: () => void unstable_getServerProps?: (context: { + params: { [key: string]: string } req: IncomingMessage res: ServerResponse query: ParsedUrlQuery }) => Promise<{ [key: string]: any }> isDataReq: boolean + params: { [key: string]: string } } function renderDocument( @@ -284,6 +286,7 @@ export async function renderToHTML( unstable_getStaticPaths, unstable_getServerProps, isDataReq, + params, } = renderOpts const callMiddleware = async (method: string, args: any[], props = false) => { @@ -339,7 +342,7 @@ export async function renderToHTML( } if (hasPageGetInitialProps && isSpr) { - throw new Error(SPR_GET_INITIAL_PROPS_CONFLICT + ` ${pathname}`) + throw new Error(SSG_GET_INITIAL_PROPS_CONFLICT + ` ${pathname}`) } if (hasPageGetInitialProps && unstable_getServerProps) { @@ -347,7 +350,7 @@ export async function renderToHTML( } if (unstable_getServerProps && isSpr) { - throw new Error(SERVER_PROPS_SPR_CONFLICT + ` ${pathname}`) + throw new Error(SERVER_PROPS_SSG_CONFLICT + ` ${pathname}`) } if (!!unstable_getStaticPaths && !isSpr) { @@ -503,6 +506,7 @@ export async function renderToHTML( if (unstable_getServerProps) { props.pageProps = await unstable_getServerProps({ + params, query, req, res, diff --git a/test/integration/getserverprops/pages/blog/[post]/index.js b/test/integration/getserverprops/pages/blog/[post]/index.js index 155a19d77f4e605..dd745965bedb250 100644 --- a/test/integration/getserverprops/pages/blog/[post]/index.js +++ b/test/integration/getserverprops/pages/blog/[post]/index.js @@ -3,30 +3,30 @@ import Link from 'next/link' import { useRouter } from 'next/router' // eslint-disable-next-line camelcase -export async function unstable_getServerProps({ query }) { - if (query.post === 'post-10') { +export async function unstable_getServerProps({ params }) { + if (params.post === 'post-10') { await new Promise(resolve => { setTimeout(() => resolve(), 1000) }) } - if (query.post === 'post-100') { + if (params.post === 'post-100') { throw new Error('such broken..') } return { - query, - post: query.post, + params, + post: params.post, time: (await import('perf_hooks')).performance.now(), } } -export default ({ post, time, query }) => { +export default ({ post, time, params }) => { return ( <>

Post: {post}

time: {time} -
{JSON.stringify(query)}
+
{JSON.stringify(params)}
{JSON.stringify(useRouter().query)}
to home diff --git a/test/integration/getserverprops/pages/something.js b/test/integration/getserverprops/pages/something.js index 94965ee53975c75..d9a3a61777a67a0 100644 --- a/test/integration/getserverprops/pages/something.js +++ b/test/integration/getserverprops/pages/something.js @@ -3,10 +3,10 @@ import Link from 'next/link' import { useRouter } from 'next/router' // eslint-disable-next-line camelcase -export async function unstable_getServerProps({ query }) { +export async function unstable_getServerProps({ params }) { return { world: 'world', - params: query || {}, + params: params || {}, time: new Date().getTime(), random: Math.random(), } diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index f9d2ad05303bdfb..a20e8caba61111b 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -164,7 +164,7 @@ const runTests = (dev = false) => { const html = await renderViaHTTP(appPort, '/blog/post-1?hello=world') const $ = cheerio.load(html) const params = $('#params').text() - expect(JSON.parse(params)).toEqual({ hello: 'world', post: 'post-1' }) + expect(JSON.parse(params)).toEqual({ post: 'post-1' }) const query = $('#query').text() expect(JSON.parse(query)).toEqual({ hello: 'world', post: 'post-1' }) }) From def15076aae4242f1b7c1a86ab900adfa0b2a1d7 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 19:05:07 -0600 Subject: [PATCH 03/23] Add no-cache header and update types --- .../next-server/server/load-components.ts | 8 +++--- .../next/next-server/server/next-server.ts | 12 ++++++--- packages/next/next-server/server/render.tsx | 25 ++++++------------- .../getserverprops/test/index.test.js | 10 ++++++++ 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/packages/next/next-server/server/load-components.ts b/packages/next/next-server/server/load-components.ts index baa01fb66d28cd5..b24a6ceb12ddb70 100644 --- a/packages/next/next-server/server/load-components.ts +++ b/packages/next/next-server/server/load-components.ts @@ -18,14 +18,16 @@ export type LoadComponentsReturnType = { Component: any pageConfig: PageConfig unstable_getStaticProps?: (params: { - params: any + params: { [key: string]: string | string[] } }) => { props: any revalidate?: number | boolean } - unstable_getStaticPaths?: () => void + unstable_getStaticPaths?: () => Promise< + Array + > unstable_getServerProps?: (context: { - params: { [key: string]: string } + params: { [key: string]: string | string[] } req: IncomingMessage res: ServerResponse query: ParsedUrlQuery diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 92dc92f3ec7eff9..9a767e38840c9ba 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -799,7 +799,9 @@ export default class Server { if (revalidate) { res.setHeader( 'Cache-Control', - `s-maxage=${revalidate}, stale-while-revalidate` + revalidate < 0 + ? `no-cache, no-store, must-revalidate` + : `s-maxage=${revalidate}, stale-while-revalidate` ) } else if (revalidate === false) { res.setHeader( @@ -829,6 +831,7 @@ export default class Server { typeof result.Component === 'object' && typeof result.Component.renderReqToHTML === 'function' const isSSG = !!result.unstable_getStaticProps + const isServerProps = !!result.unstable_getServerProps // Toggle whether or not this is a Data request const isDataReq = query._nextDataReq @@ -856,7 +859,8 @@ export default class Server { this.__sendPayload( res, JSON.stringify(renderResult?.renderOpts?.pageData), - 'application/json' + 'application/json', + -1 ) return null } @@ -872,13 +876,13 @@ export default class Server { return result.Component.renderReqToHTML(req, res) } - if (isDataReq && typeof result.unstable_getServerProps === 'function') { + if (isDataReq && isServerProps) { const props = await renderToHTML(req, res, pathname, query, { ...result, ...opts, isDataReq, }) - this.__sendPayload(res, JSON.stringify(props), 'application/json') + this.__sendPayload(res, JSON.stringify(props), 'application/json', -1) return null } diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 61ee13b683fcd21..1809a6288f9272b 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -34,6 +34,7 @@ import { SERVER_PROPS_SSG_CONFLICT, } from '../../lib/constants' import { AMP_RENDER_TARGET } from '../lib/constants' +import { LoadComponentsReturnType } from './load-components' export type ManifestItem = { id: number | string @@ -126,7 +127,7 @@ function render( return { html, head } } -type RenderOpts = { +type RenderOpts = LoadComponentsReturnType & { documentMiddlewareEnabled: boolean staticMarkup: boolean buildId: string @@ -152,19 +153,6 @@ type RenderOpts = { App: AppType ErrorDebug?: React.ComponentType<{ error: Error }> ampValidator?: (html: string, pathname: string) => Promise - unstable_getStaticProps?: (params: { - params: any | undefined - }) => Promise<{ - props: any - revalidate?: number | boolean - }> - unstable_getStaticPaths?: () => void - unstable_getServerProps?: (context: { - params: { [key: string]: string } - req: IncomingMessage - res: ServerResponse - query: ParsedUrlQuery - }) => Promise<{ [key: string]: any }> isDataReq: boolean params: { [key: string]: string } } @@ -447,7 +435,7 @@ export async function renderToHTML( if (isSpr) { const data = await unstable_getStaticProps!({ - params: isDynamicRoute(pathname) ? query : undefined, + params: isDynamicRoute(pathname) ? (query as any) : undefined, }) const invalidKeys = Object.keys(data).filter( @@ -569,7 +557,10 @@ export async function renderToHTML( ) } const documentCtx = { ...ctx, renderPage } - const docProps = await loadGetInitialProps(Document, documentCtx) + const docProps: DocumentInitialProps = await loadGetInitialProps( + Document, + documentCtx + ) // the response might be finished on the getInitialProps call if (isResSent(res) && !isSpr) return null @@ -584,7 +575,7 @@ export async function renderToHTML( const dynamicImports: ManifestItem[] = [] for (const mod of reactLoadableModules) { - const manifestItem = reactLoadableManifest[mod] + const manifestItem: ManifestItem[] = reactLoadableManifest[mod] if (manifestItem) { manifestItem.forEach(item => { diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index a20e8caba61111b..03fcd07404ab985 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -260,6 +260,16 @@ const runTests = (dev = false) => { expect(routesManifest.serverPropsRoutes).toEqual(expectedManifestRoutes()) }) + + it('should set no-cache, no-store, must-revalidate header', async () => { + const res = await fetchViaHTTP( + appPort, + `/_next/data/${buildId}/something.json` + ) + expect(res.headers.get('cache-control')).toBe( + 'no-cache, no-store, must-revalidate' + ) + }) } } From 40b3e68b3df028cd3c387bd5bbcc20225de98179 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 19:15:06 -0600 Subject: [PATCH 04/23] Revert sharing of load-components type --- packages/next/next-server/server/render.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 1809a6288f9272b..dcbb6586d196299 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -34,7 +34,6 @@ import { SERVER_PROPS_SSG_CONFLICT, } from '../../lib/constants' import { AMP_RENDER_TARGET } from '../lib/constants' -import { LoadComponentsReturnType } from './load-components' export type ManifestItem = { id: number | string @@ -127,7 +126,7 @@ function render( return { html, head } } -type RenderOpts = LoadComponentsReturnType & { +type RenderOpts = { documentMiddlewareEnabled: boolean staticMarkup: boolean buildId: string @@ -155,6 +154,21 @@ type RenderOpts = LoadComponentsReturnType & { ampValidator?: (html: string, pathname: string) => Promise isDataReq: boolean params: { [key: string]: string } + unstable_getStaticProps?: (params: { + params: { [key: string]: string | string[] } + }) => { + props: any + revalidate?: number | boolean + } + unstable_getStaticPaths?: () => Promise< + Array + > + unstable_getServerProps?: (context: { + params: { [key: string]: string | string[] } + req: IncomingMessage + res: ServerResponse + query: ParsedUrlQuery + }) => Promise<{ [key: string]: any }> } function renderDocument( From 7397571d38d20f571a7d5b0ca37e2cc240640bfd Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 20:26:38 -0600 Subject: [PATCH 05/23] Add catchall test and update routes-manifest field --- packages/next/build/index.ts | 17 ++- .../pages/catchall/[...path].js | 32 ++++++ .../getserverprops/test/index.test.js | 107 ++++++++++++++---- 3 files changed, 126 insertions(+), 30 deletions(-) create mode 100644 test/integration/getserverprops/pages/catchall/[...path].js diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 10e4c5e6fa88246..9e1ba381d47a8a4 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -530,13 +530,20 @@ export default async function build(dir: string, conf = null): Promise { routesManifest.serverPropsRoutes = {} for (const page of serverPropsPages) { + const dataRoute = path.posix.join( + '/_next/data', + buildId, + `${page === '/' ? '/index' : page}.json` + ) + routesManifest.serverPropsRoutes[page] = { page, - dataRoute: path.posix.join( - '/_next/data', - buildId, - `${page === '/' ? '/index' : page}.json` - ), + dataRouteRegex: isDynamicRoute(page) + ? getRouteRegex(dataRoute.replace(/\.json$/, '')).re.source.replace( + /\(\?:\\\/\)\?\$$/, + '\\.json$' + ) + : new RegExp(`^${dataRoute}$`).source, } } diff --git a/test/integration/getserverprops/pages/catchall/[...path].js b/test/integration/getserverprops/pages/catchall/[...path].js new file mode 100644 index 000000000000000..d9a3a61777a67a0 --- /dev/null +++ b/test/integration/getserverprops/pages/catchall/[...path].js @@ -0,0 +1,32 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps({ params }) { + return { + world: 'world', + params: params || {}, + time: new Date().getTime(), + random: Math.random(), + } +} + +export default ({ world, time, params, random }) => { + return ( + <> +

hello: {world}

+ time: {time} +
{random}
+
{JSON.stringify(params)}
+
{JSON.stringify(useRouter().query)}
+ + to home + +
+ + to another + + + ) +} diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index 03fcd07404ab985..e2201ac80d5c259 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -13,6 +13,7 @@ import { waitFor, nextBuild, nextStart, + normalizeRegEx, } from 'next-test-utils' jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2 @@ -22,38 +23,66 @@ let app let appPort let buildId +const escapeRegex = str => str.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&') + const expectedManifestRoutes = () => ({ - '/user/[user]/profile': { - dataRoute: `/_next/data/${buildId}/user/[user]/profile.json`, - page: '/user/[user]/profile', + '/something': { + page: '/something', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/something.json$` + ), + }, + '/blog/[post]': { + page: '/blog/[post]', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/blog\\/([^/]+?)\\.json$` + ), }, '/': { - dataRoute: `/_next/data/${buildId}/index.json`, page: '/', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/index.json$` + ), }, - '/blog/[post]/[comment]': { - dataRoute: `/_next/data/${buildId}/blog/[post]/[comment].json`, - page: '/blog/[post]/[comment]', + '/default-revalidate': { + page: '/default-revalidate', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/default-revalidate.json$` + ), + }, + '/catchall/[...path]': { + page: '/catchall/[...path]', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/catchall\\/(.+?)\\.json$` + ), }, '/blog': { - dataRoute: `/_next/data/${buildId}/blog.json`, page: '/blog', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/blog.json$` + ), }, - '/default-revalidate': { - dataRoute: `/_next/data/${buildId}/default-revalidate.json`, - page: '/default-revalidate', + '/blog/[post]/[comment]': { + page: '/blog/[post]/[comment]', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex( + buildId + )}\\/blog\\/([^/]+?)\\/([^/]+?)\\.json$` + ), + }, + '/user/[user]/profile': { + page: '/user/[user]/profile', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex( + buildId + )}\\/user\\/([^/]+?)\\/profile\\.json$` + ), }, '/another': { - dataRoute: `/_next/data/${buildId}/another.json`, page: '/another', - }, - '/blog/[post]': { - dataRoute: `/_next/data/${buildId}/blog/[post].json`, - page: '/blog/[post]', - }, - '/something': { - dataRoute: `/_next/data/${buildId}/something.json`, - page: '/something', + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/another.json$` + ), }, }) @@ -169,16 +198,40 @@ const runTests = (dev = false) => { expect(JSON.parse(query)).toEqual({ hello: 'world', post: 'post-1' }) }) + it('should supply params values for catchall correctly', async () => { + const html = await renderViaHTTP(appPort, '/catchall/first') + const $ = cheerio.load(html) + const params = $('#params').text() + expect(JSON.parse(params)).toEqual({ path: ['first'] }) + const query = $('#query').text() + expect(JSON.parse(query)).toEqual({ path: ['first'] }) + + const data = JSON.parse( + await renderViaHTTP( + appPort, + `/_next/data/${escapeRegex(buildId)}/catchall/first.json` + ) + ) + + expect(data.pageProps.params).toEqual({ path: ['first'] }) + }) + it('should return data correctly', async () => { const data = JSON.parse( - await renderViaHTTP(appPort, `/_next/data/${buildId}/something.json`) + await renderViaHTTP( + appPort, + `/_next/data/${escapeRegex(buildId)}/something.json` + ) ) expect(data.pageProps.world).toBe('world') }) it('should return data correctly for dynamic page', async () => { const data = JSON.parse( - await renderViaHTTP(appPort, `/_next/data/${buildId}/blog/post-1.json`) + await renderViaHTTP( + appPort, + `/_next/data/${escapeRegex(buildId)}/blog/post-1.json` + ) ) expect(data.pageProps.post).toBe('post-1') }) @@ -254,17 +307,21 @@ const runTests = (dev = false) => { }) it('should output routes-manifest correctly', async () => { - const routesManifest = await fs.readJSON( + const { serverPropsRoutes } = await fs.readJSON( join(appDir, '.next/routes-manifest.json') ) + for (const key of Object.keys(serverPropsRoutes)) { + const val = serverPropsRoutes[key].dataRouteRegex + serverPropsRoutes[key].dataRouteRegex = normalizeRegEx(val) + } - expect(routesManifest.serverPropsRoutes).toEqual(expectedManifestRoutes()) + expect(serverPropsRoutes).toEqual(expectedManifestRoutes()) }) it('should set no-cache, no-store, must-revalidate header', async () => { const res = await fetchViaHTTP( appPort, - `/_next/data/${buildId}/something.json` + `/_next/data/${escapeRegex(buildId)}/something.json` ) expect(res.headers.get('cache-control')).toBe( 'no-cache, no-store, must-revalidate' From 25be5ef03df57cec5feb152bc19c9f21ff628f18 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 20:59:44 -0600 Subject: [PATCH 06/23] Update header check --- test/integration/getserverprops/test/index.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index e2201ac80d5c259..15ce5f47b3e82fe 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -323,9 +323,7 @@ const runTests = (dev = false) => { appPort, `/_next/data/${escapeRegex(buildId)}/something.json` ) - expect(res.headers.get('cache-control')).toBe( - 'no-cache, no-store, must-revalidate' - ) + expect(res.headers.get('cache-control')).toContain('no-cache') }) } } From 83c89a872eb053d4ba8618b6c363d643b8862697 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 17 Jan 2020 11:36:28 -0600 Subject: [PATCH 07/23] Update to pass query for getServerProps data requests --- packages/next/build/index.ts | 7 +++- .../webpack/loaders/next-serverless-loader.ts | 7 ++-- .../next/next-server/lib/router/router.ts | 9 +++-- .../next/next-server/server/next-server.ts | 7 ++-- .../integration/getserverprops/pages/index.js | 3 ++ .../getserverprops/pages/something.js | 6 ++-- .../getserverprops/test/index.test.js | 33 +++++++++++++------ 7 files changed, 51 insertions(+), 21 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 9e1ba381d47a8a4..a146626055e7e2b 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -530,6 +530,11 @@ export default async function build(dir: string, conf = null): Promise { routesManifest.serverPropsRoutes = {} for (const page of serverPropsPages) { + const cleanDataRoute = path.posix.join( + '/_next/data', + buildId.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'), + `${page === '/' ? '/index' : page}.json` + ) const dataRoute = path.posix.join( '/_next/data', buildId, @@ -543,7 +548,7 @@ export default async function build(dir: string, conf = null): Promise { /\(\?:\\\/\)\?\$$/, '\\.json$' ) - : new RegExp(`^${dataRoute}$`).source, + : new RegExp(`^${cleanDataRoute}$`).source, } } diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 00d0238b3504f9b..ff34be1b045b213 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -217,13 +217,14 @@ const nextServerlessLoader: loader.Loader = function() { } let _nextData = false - if (req.url.match(/_next\\/data/)) { + const parsedUrl = handleRewrites(parse(req.url, true)) + + if (parsedUrl.pathname.match(/_next\\/data/)) { _nextData = true - req.url = req.url + parsedUrl.pathname = parsedUrl.pathname .replace(new RegExp('/_next/data/${escapedBuildId}/'), '/') .replace(/\\.json$/, '') } - const parsedUrl = handleRewrites(parse(req.url, true)) const renderOpts = Object.assign( { diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 508b941e191bbad..96fd454257ffc13 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -665,15 +665,18 @@ export default class Router implements BaseRouter { } _getStaticData = (asPath: string, _cachedData?: object): Promise => { - let pathname = parse(asPath).pathname + let { pathname, query } = parse(asPath, true) pathname = !pathname || pathname === '/' ? '/index' : pathname return process.env.NODE_ENV === 'production' && (_cachedData = this.sdc[pathname]) ? Promise.resolve(_cachedData) : fetch( - // @ts-ignore __NEXT_DATA__ - `/_next/data/${__NEXT_DATA__.buildId}${pathname}.json` + formatWithValidation({ + // @ts-ignore __NEXT_DATA__ + pathname: `/_next/data/${__NEXT_DATA__.buildId}${pathname}.json`, + query, + }) ) .then(res => { if (!res.ok) { diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 9a767e38840c9ba..04e6be227726b10 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -380,7 +380,7 @@ export default class Server { req, res, pathname, - { _nextDataReq: '1' }, + { ..._parsedUrl.query, _nextDataReq: '1' }, parsedUrl ) return { @@ -842,7 +842,10 @@ export default class Server { if (isLikeServerless && isDataReq) { let { pathname } = parseUrl(req.url || '', true) pathname = !pathname || pathname === '/' ? '/index' : pathname - req.url = `/_next/data/${this.buildId}${pathname}.json` + req.url = formatUrl({ + pathname: `/_next/data/${this.buildId}${pathname}.json`, + query, + }) } // non-spr requests should render like normal diff --git a/test/integration/getserverprops/pages/index.js b/test/integration/getserverprops/pages/index.js index f41bb023011461f..8266c9c60e81ffb 100644 --- a/test/integration/getserverprops/pages/index.js +++ b/test/integration/getserverprops/pages/index.js @@ -35,6 +35,9 @@ const Page = ({ world, time }) => { to another dynamic + + to something?another=thing + ) } diff --git a/test/integration/getserverprops/pages/something.js b/test/integration/getserverprops/pages/something.js index d9a3a61777a67a0..6eeba67fb6a7730 100644 --- a/test/integration/getserverprops/pages/something.js +++ b/test/integration/getserverprops/pages/something.js @@ -3,22 +3,24 @@ import Link from 'next/link' import { useRouter } from 'next/router' // eslint-disable-next-line camelcase -export async function unstable_getServerProps({ params }) { +export async function unstable_getServerProps({ params, query }) { return { world: 'world', + query: query || {}, params: params || {}, time: new Date().getTime(), random: Math.random(), } } -export default ({ world, time, params, random }) => { +export default ({ world, time, params, random, query }) => { return ( <>

hello: {world}

time: {time}
{random}
{JSON.stringify(params)}
+
{JSON.stringify(query)}
{JSON.stringify(useRouter().query)}
to home diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index 15ce5f47b3e82fe..4c0a30b99ceb394 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -207,10 +207,7 @@ const runTests = (dev = false) => { expect(JSON.parse(query)).toEqual({ path: ['first'] }) const data = JSON.parse( - await renderViaHTTP( - appPort, - `/_next/data/${escapeRegex(buildId)}/catchall/first.json` - ) + await renderViaHTTP(appPort, `/_next/data/${buildId}/catchall/first.json`) ) expect(data.pageProps.params).toEqual({ path: ['first'] }) @@ -218,21 +215,25 @@ const runTests = (dev = false) => { it('should return data correctly', async () => { const data = JSON.parse( - await renderViaHTTP( - appPort, - `/_next/data/${escapeRegex(buildId)}/something.json` - ) + await renderViaHTTP(appPort, `/_next/data/${buildId}/something.json`) ) expect(data.pageProps.world).toBe('world') }) - it('should return data correctly for dynamic page', async () => { + it('should pass query for data request', async () => { const data = JSON.parse( await renderViaHTTP( appPort, - `/_next/data/${escapeRegex(buildId)}/blog/post-1.json` + `/_next/data/${buildId}/something.json?another=thing` ) ) + expect(data.pageProps.query.another).toBe('thing') + }) + + it('should return data correctly for dynamic page', async () => { + const data = JSON.parse( + await renderViaHTTP(appPort, `/_next/data/${buildId}/blog/post-1.json`) + ) expect(data.pageProps.post).toBe('post-1') }) @@ -255,6 +256,18 @@ const runTests = (dev = false) => { expect(text).toMatch(/post.*?post-1/) }) + it('should pass query for data request on navigation', async () => { + const browser = await webdriver(appPort, '/') + await browser.eval('window.beforeNav = true') + await browser.elementByCss('#something-query').click() + await browser.waitForElementByCss('#initial-query') + const query = JSON.parse( + await browser.elementByCss('#initial-query').text() + ) + expect(await browser.eval('window.beforeNav')).toBe(true) + expect(query.another).toBe('thing') + }) + it('should reload page on failed data request', async () => { const browser = await webdriver(appPort, '/') await waitFor(500) From 9a5bf527b6a6f591b9abc9ffa40739a59fc04b20 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 17 Jan 2020 12:04:32 -0600 Subject: [PATCH 08/23] Update to not cache getServerProps requests --- .../build/babel/plugins/next-ssg-transform.ts | 31 +++++++++++-------- .../next/next-server/lib/router/router.ts | 17 +++++----- .../getserverprops/test/index.test.js | 6 +--- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/packages/next/build/babel/plugins/next-ssg-transform.ts b/packages/next/build/babel/plugins/next-ssg-transform.ts index 98c08c8058a1178..185ba2bf7bcca7b 100644 --- a/packages/next/build/babel/plugins/next-ssg-transform.ts +++ b/packages/next/build/babel/plugins/next-ssg-transform.ts @@ -3,6 +3,7 @@ import * as BabelTypes from '@babel/types' const pageComponentVar = '__NEXT_COMP' const prerenderId = '__N_SSG' +const serverPropsId = '__N_SPROPS' export const EXPORT_NAME_GET_STATIC_PROPS = 'unstable_getStaticProps' export const EXPORT_NAME_GET_STATIC_PATHS = 'unstable_getStaticPaths' @@ -17,6 +18,7 @@ const ssgExports = new Set([ type PluginState = { refs: Set> isPrerender: boolean + isServerProps: boolean done: boolean } @@ -46,7 +48,7 @@ function decorateSsgExport( '=', t.memberExpression( t.identifier(pageComponentVar), - t.identifier(prerenderId) + t.identifier(state.isServerProps ? serverPropsId : prerenderId) ), t.booleanLiteral(true) ), @@ -57,6 +59,17 @@ function decorateSsgExport( }) } +const checkIsSSG = (name: string, state: PluginState, toRemove: any) => { + if (ssgExports.has(name)) { + state.isPrerender = true + + if (name === EXPORT_NAME_GET_SERVER_PROPS) { + state.isServerProps = true + } + toRemove.remove() + } +} + export default function nextTransformSsg({ types: t, }: { @@ -136,6 +149,7 @@ export default function nextTransformSsg({ enter(_, state) { state.refs = new Set>() state.isPrerender = false + state.isServerProps = false state.done = false }, exit(path, state) { @@ -241,10 +255,7 @@ export default function nextTransformSsg({ const specifiers = path.get('specifiers') if (specifiers.length) { specifiers.forEach(s => { - if (ssgExports.has(s.node.exported.name)) { - state.isPrerender = true - s.remove() - } + checkIsSSG(s.node.exported.name, state, s) }) if (path.node.specifiers.length < 1) { @@ -261,10 +272,7 @@ export default function nextTransformSsg({ switch (decl.node.type) { case 'FunctionDeclaration': { const name = decl.node.id!.name - if (ssgExports.has(name)) { - state.isPrerender = true - path.remove() - } + checkIsSSG(name, state, path) break } case 'VariableDeclaration': { @@ -276,10 +284,7 @@ export default function nextTransformSsg({ return } const name = d.node.id.name - if (ssgExports.has(name)) { - state.isPrerender = true - d.remove() - } + checkIsSSG(name, state, d) }) break } diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index aca64958f590339..1feba14b88e44f9 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -449,9 +449,11 @@ export default class Router implements BaseRouter { } } + const isServerProps = (Component as any).__N_SPROPS + return this._getData(() => - (Component as any).__N_SSG - ? this._getStaticData(as) + (Component as any).__N_SSG || isServerProps + ? this._getStaticData(as, !isServerProps) : this.getInitialProps( Component, // we provide AppTree later so this needs to be `any` @@ -664,13 +666,12 @@ export default class Router implements BaseRouter { }) } - _getStaticData = (asPath: string, _cachedData?: object): Promise => { + _getStaticData = (asPath: string, cache?: boolean): Promise => { let { pathname, query } = parse(asPath, true) pathname = toRoute(!pathname || pathname === '/' ? '/index' : pathname) - return process.env.NODE_ENV === 'production' && - (_cachedData = this.sdc[pathname]) - ? Promise.resolve(_cachedData) + return process.env.NODE_ENV === 'production' && this.sdc[pathname] + ? Promise.resolve(this.sdc[pathname]) : fetch( formatWithValidation({ // @ts-ignore __NEXT_DATA__ @@ -685,7 +686,9 @@ export default class Router implements BaseRouter { return res.json() }) .then(data => { - this.sdc[pathname!] = data + if (cache) { + this.sdc[pathname!] = data + } return data }) .catch((err: Error) => { diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index 4c0a30b99ceb394..a24483996b2ac9e 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -134,11 +134,7 @@ const navigateTest = (dev = false) => { await goFromHomeToAnother() const nextTime = await browser.elementByCss('#anotherTime').text() - if (dev) { - expect(snapTime).not.toMatch(nextTime) - } else { - expect(snapTime).toMatch(nextTime) - } + expect(snapTime).not.toMatch(nextTime) // Reset to Home for next test await goFromAnotherToHome() From 931d86c3799f4b168683fbe46b97913a99000427 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sat, 18 Jan 2020 20:40:14 -0600 Subject: [PATCH 09/23] Rename server side props identifier --- packages/next/build/babel/plugins/next-ssg-transform.ts | 2 +- packages/next/next-server/lib/router/router.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/build/babel/plugins/next-ssg-transform.ts b/packages/next/build/babel/plugins/next-ssg-transform.ts index 185ba2bf7bcca7b..f4523ab05505690 100644 --- a/packages/next/build/babel/plugins/next-ssg-transform.ts +++ b/packages/next/build/babel/plugins/next-ssg-transform.ts @@ -3,7 +3,7 @@ import * as BabelTypes from '@babel/types' const pageComponentVar = '__NEXT_COMP' const prerenderId = '__N_SSG' -const serverPropsId = '__N_SPROPS' +const serverPropsId = '__N_SSP' export const EXPORT_NAME_GET_STATIC_PROPS = 'unstable_getStaticProps' export const EXPORT_NAME_GET_STATIC_PATHS = 'unstable_getStaticPaths' diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index fdbbfa2a72765fb..088955dd257e904 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -449,7 +449,7 @@ export default class Router implements BaseRouter { } } - const isServerProps = (Component as any).__N_SPROPS + const isServerProps = (Component as any).__N_SSP return this._getData(() => (Component as any).__N_SSG || isServerProps From d456df1c98a260a7ea9dbbf898fc6b0ddf99d08f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 22 Jan 2020 10:22:49 -0600 Subject: [PATCH 10/23] Update to nest props for getServerProps --- packages/next/next-server/server/render.tsx | 24 +++++++++---- .../getserverprops/pages/another/index.js | 6 ++-- .../pages/blog/[post]/[comment].js | 8 +++-- .../getserverprops/pages/blog/[post]/index.js | 8 +++-- .../getserverprops/pages/blog/index.js | 6 ++-- .../pages/catchall/[...path].js | 10 +++--- .../pages/default-revalidate.js | 6 ++-- .../integration/getserverprops/pages/index.js | 6 ++-- .../getserverprops/pages/invalid-keys.js | 34 +++++++++++++++++++ .../getserverprops/pages/something.js | 12 ++++--- .../pages/user/[user]/profile.js | 6 ++-- .../getserverprops/test/index.test.js | 18 +++++++++- 12 files changed, 112 insertions(+), 32 deletions(-) create mode 100644 test/integration/getserverprops/pages/invalid-keys.js diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index dcbb6586d196299..041959cd7da1bcd 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -439,6 +439,14 @@ export async function renderToHTML( ) + const invalidKeysMsg = (methodName: string, invalidKeys: string[]) => { + throw new Error( + `Additional keys were returned from \`${methodName}\`. Properties intended for your component must be nested under the \`props\` key, e.g.:` + + `\n\n\treturn { props: { title: 'My Title', content: '...' } }` + + `\n\nKeys that need to be moved: ${invalidKeys.join(', ')}.` + ) + } + try { props = await loadGetInitialProps(App, { AppTree: ctx.AppTree, @@ -457,11 +465,7 @@ export async function renderToHTML( ) if (invalidKeys.length) { - throw new Error( - `Additional keys were returned from \`getStaticProps\`. Properties intended for your component must be nested under the \`props\` key, e.g.:` + - `\n\n\treturn { props: { title: 'My Title', content: '...' } }` + - `\n\nKeys that need to be moved: ${invalidKeys.join(', ')}.` - ) + invalidKeysMsg('getStaticProps', invalidKeys) } if (typeof data.revalidate === 'number') { @@ -507,12 +511,20 @@ export async function renderToHTML( } if (unstable_getServerProps) { - props.pageProps = await unstable_getServerProps({ + const data = await unstable_getServerProps({ params, query, req, res, }) + + const invalidKeys = Object.keys(data).filter(key => key !== 'props') + + if (invalidKeys.length) { + invalidKeysMsg('getServerProps', invalidKeys) + } + + props.pageProps = data.props ;(renderOpts as any).pageData = props } // We only need to do this if we want to support calling diff --git a/test/integration/getserverprops/pages/another/index.js b/test/integration/getserverprops/pages/another/index.js index 7f588dcf3a876fe..a5c3afb6343c63f 100644 --- a/test/integration/getserverprops/pages/another/index.js +++ b/test/integration/getserverprops/pages/another/index.js @@ -16,8 +16,10 @@ export async function unstable_getServerProps() { .trim() return { - world: text, - time: new Date().getTime(), + props: { + world: text, + time: new Date().getTime(), + }, } } diff --git a/test/integration/getserverprops/pages/blog/[post]/[comment].js b/test/integration/getserverprops/pages/blog/[post]/[comment].js index 840676c6f756b70..292c98024942dc3 100644 --- a/test/integration/getserverprops/pages/blog/[post]/[comment].js +++ b/test/integration/getserverprops/pages/blog/[post]/[comment].js @@ -4,9 +4,11 @@ import Link from 'next/link' // eslint-disable-next-line camelcase export async function unstable_getServerProps({ query }) { return { - post: query.post, - comment: query.comment, - time: new Date().getTime(), + props: { + post: query.post, + comment: query.comment, + time: new Date().getTime(), + }, } } diff --git a/test/integration/getserverprops/pages/blog/[post]/index.js b/test/integration/getserverprops/pages/blog/[post]/index.js index dd745965bedb250..aa4e0c5cc2bd12a 100644 --- a/test/integration/getserverprops/pages/blog/[post]/index.js +++ b/test/integration/getserverprops/pages/blog/[post]/index.js @@ -15,9 +15,11 @@ export async function unstable_getServerProps({ params }) { } return { - params, - post: params.post, - time: (await import('perf_hooks')).performance.now(), + props: { + params, + post: params.post, + time: (await import('perf_hooks')).performance.now(), + }, } } diff --git a/test/integration/getserverprops/pages/blog/index.js b/test/integration/getserverprops/pages/blog/index.js index 341930d994fc23b..056a1860a3e3dfa 100644 --- a/test/integration/getserverprops/pages/blog/index.js +++ b/test/integration/getserverprops/pages/blog/index.js @@ -4,8 +4,10 @@ import Link from 'next/link' // eslint-disable-next-line camelcase export async function unstable_getServerProps() { return { - slugs: ['post-1', 'post-2'], - time: (await import('perf_hooks')).performance.now(), + props: { + slugs: ['post-1', 'post-2'], + time: (await import('perf_hooks')).performance.now(), + }, } } diff --git a/test/integration/getserverprops/pages/catchall/[...path].js b/test/integration/getserverprops/pages/catchall/[...path].js index d9a3a61777a67a0..5e1bd3543f63fec 100644 --- a/test/integration/getserverprops/pages/catchall/[...path].js +++ b/test/integration/getserverprops/pages/catchall/[...path].js @@ -5,10 +5,12 @@ import { useRouter } from 'next/router' // eslint-disable-next-line camelcase export async function unstable_getServerProps({ params }) { return { - world: 'world', - params: params || {}, - time: new Date().getTime(), - random: Math.random(), + props: { + world: 'world', + params: params || {}, + time: new Date().getTime(), + random: Math.random(), + }, } } diff --git a/test/integration/getserverprops/pages/default-revalidate.js b/test/integration/getserverprops/pages/default-revalidate.js index d078e6a08e8a875..15bb81ba617e876 100644 --- a/test/integration/getserverprops/pages/default-revalidate.js +++ b/test/integration/getserverprops/pages/default-revalidate.js @@ -3,8 +3,10 @@ import Link from 'next/link' // eslint-disable-next-line camelcase export async function unstable_getServerProps() { return { - world: 'world', - time: new Date().getTime(), + props: { + world: 'world', + time: new Date().getTime(), + }, } } diff --git a/test/integration/getserverprops/pages/index.js b/test/integration/getserverprops/pages/index.js index 8266c9c60e81ffb..f906758f846b1d8 100644 --- a/test/integration/getserverprops/pages/index.js +++ b/test/integration/getserverprops/pages/index.js @@ -3,8 +3,10 @@ import Link from 'next/link' // eslint-disable-next-line camelcase export async function unstable_getServerProps() { return { - world: 'world', - time: new Date().getTime(), + props: { + world: 'world', + time: new Date().getTime(), + }, } } diff --git a/test/integration/getserverprops/pages/invalid-keys.js b/test/integration/getserverprops/pages/invalid-keys.js new file mode 100644 index 000000000000000..6eeba67fb6a7730 --- /dev/null +++ b/test/integration/getserverprops/pages/invalid-keys.js @@ -0,0 +1,34 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +// eslint-disable-next-line camelcase +export async function unstable_getServerProps({ params, query }) { + return { + world: 'world', + query: query || {}, + params: params || {}, + time: new Date().getTime(), + random: Math.random(), + } +} + +export default ({ world, time, params, random, query }) => { + return ( + <> +

hello: {world}

+ time: {time} +
{random}
+
{JSON.stringify(params)}
+
{JSON.stringify(query)}
+
{JSON.stringify(useRouter().query)}
+ + to home + +
+ + to another + + + ) +} diff --git a/test/integration/getserverprops/pages/something.js b/test/integration/getserverprops/pages/something.js index 6eeba67fb6a7730..f3d5ef1ef4de2f0 100644 --- a/test/integration/getserverprops/pages/something.js +++ b/test/integration/getserverprops/pages/something.js @@ -5,11 +5,13 @@ import { useRouter } from 'next/router' // eslint-disable-next-line camelcase export async function unstable_getServerProps({ params, query }) { return { - world: 'world', - query: query || {}, - params: params || {}, - time: new Date().getTime(), - random: Math.random(), + props: { + world: 'world', + query: query || {}, + params: params || {}, + time: new Date().getTime(), + random: Math.random(), + }, } } diff --git a/test/integration/getserverprops/pages/user/[user]/profile.js b/test/integration/getserverprops/pages/user/[user]/profile.js index 722fbc24671e388..a4fb1e13403bf04 100644 --- a/test/integration/getserverprops/pages/user/[user]/profile.js +++ b/test/integration/getserverprops/pages/user/[user]/profile.js @@ -4,8 +4,10 @@ import Link from 'next/link' // eslint-disable-next-line camelcase export async function unstable_getServerProps({ query }) { return { - user: query.user, - time: (await import('perf_hooks')).performance.now(), + props: { + user: query.user, + time: (await import('perf_hooks')).performance.now(), + }, } } diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index a24483996b2ac9e..992d1d0b75fe660 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -84,6 +84,12 @@ const expectedManifestRoutes = () => ({ `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/another.json$` ), }, + '/invalid-keys': { + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/invalid-keys.json$` + ), + page: '/invalid-keys', + }, }) const navigateTest = (dev = false) => { @@ -306,7 +312,17 @@ const runTests = (dev = false) => { expect(curRandom).toBe(initialRandom + '') }) - if (!dev) { + if (dev) { + it('should show error for extra keys returned from getServerProps', async () => { + const html = await renderViaHTTP(appPort, '/invalid-keys') + expect(html).toContain( + `Additional keys were returned from \`getServerProps\`. Properties intended for your component must be nested under the \`props\` key, e.g.:` + ) + expect(html).toContain( + `Keys that need to be moved: world, query, params, time, random` + ) + }) + } else { it('should not fetch data on mount', async () => { const browser = await webdriver(appPort, '/blog/post-100') await browser.eval('window.thisShouldStay = true') From 1ad34d75e01d720248e9bb4730a4f930a82758f5 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 16:23:20 -0600 Subject: [PATCH 11/23] Add no-cache header in serverless-loader also --- packages/next/build/webpack/loaders/next-serverless-loader.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index ff34be1b045b213..27fbedd28b4111c 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -286,7 +286,9 @@ const nextServerlessLoader: loader.Loader = function() { if (renderOpts.revalidate) { res.setHeader( 'Cache-Control', - \`s-maxage=\${renderOpts.revalidate}, stale-while-revalidate\` + renderOpts.revalidate < 0 + ? \`no-cache, no-store, must-revalidate\` + : \`s-maxage=\${renderOpts.revalidate}, stale-while-revalidate\` ) } res.end(payload) From 7a1cc60b03c0c7843375b11e9ce36a3508e03971 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 16:38:44 -0600 Subject: [PATCH 12/23] Update to throw error for mixed SSG/serverProps earlier --- .../build/babel/plugins/next-ssg-transform.ts | 15 ++++++--- .../pages/index.js | 13 ++++++++ .../pages/index.js.alt | 13 ++++++++ .../test/index.test.js | 32 +++++++++++++++++++ 4 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 test/integration/mixed-ssg-serverprops-error/pages/index.js create mode 100644 test/integration/mixed-ssg-serverprops-error/pages/index.js.alt create mode 100644 test/integration/mixed-ssg-serverprops-error/test/index.test.js diff --git a/packages/next/build/babel/plugins/next-ssg-transform.ts b/packages/next/build/babel/plugins/next-ssg-transform.ts index f4523ab05505690..a93b2a9e26372af 100644 --- a/packages/next/build/babel/plugins/next-ssg-transform.ts +++ b/packages/next/build/babel/plugins/next-ssg-transform.ts @@ -1,5 +1,6 @@ import { NodePath, PluginObj } from '@babel/core' import * as BabelTypes from '@babel/types' +import { SERVER_PROPS_SSG_CONFLICT } from '../../../lib/constants' const pageComponentVar = '__NEXT_COMP' const prerenderId = '__N_SSG' @@ -48,7 +49,7 @@ function decorateSsgExport( '=', t.memberExpression( t.identifier(pageComponentVar), - t.identifier(state.isServerProps ? serverPropsId : prerenderId) + t.identifier(state.isPrerender ? prerenderId : serverPropsId) ), t.booleanLiteral(true) ), @@ -59,12 +60,18 @@ function decorateSsgExport( }) } +const mixedSsgServerPropsError = () => { + throw new Error(SERVER_PROPS_SSG_CONFLICT) +} + const checkIsSSG = (name: string, state: PluginState, toRemove: any) => { if (ssgExports.has(name)) { - state.isPrerender = true - if (name === EXPORT_NAME_GET_SERVER_PROPS) { + if (state.isPrerender) mixedSsgServerPropsError() state.isServerProps = true + } else { + if (state.isServerProps) mixedSsgServerPropsError() + state.isPrerender = true } toRemove.remove() } @@ -153,7 +160,7 @@ export default function nextTransformSsg({ state.done = false }, exit(path, state) { - if (!state.isPrerender) { + if (!state.isPrerender && !state.isServerProps) { return } diff --git a/test/integration/mixed-ssg-serverprops-error/pages/index.js b/test/integration/mixed-ssg-serverprops-error/pages/index.js new file mode 100644 index 000000000000000..dedf3d00be14bed --- /dev/null +++ b/test/integration/mixed-ssg-serverprops-error/pages/index.js @@ -0,0 +1,13 @@ +export const unstable_getStaticProps = async () => { + return { + props: { world: 'world' }, + } +} + +export const unstable_getServerProps = async () => { + return { + props: { world: 'world' }, + } +} + +export default ({ world }) =>

Hello {world}

diff --git a/test/integration/mixed-ssg-serverprops-error/pages/index.js.alt b/test/integration/mixed-ssg-serverprops-error/pages/index.js.alt new file mode 100644 index 000000000000000..910eef220630d25 --- /dev/null +++ b/test/integration/mixed-ssg-serverprops-error/pages/index.js.alt @@ -0,0 +1,13 @@ +export const unstable_getStaticPaths = async () => { + return { + props: { world: 'world' } + } +} + +export const unstable_getServerProps = async () => { + return { + props: { world: 'world' } + } +} + +export default ({ world }) =>

Hello {world}

diff --git a/test/integration/mixed-ssg-serverprops-error/test/index.test.js b/test/integration/mixed-ssg-serverprops-error/test/index.test.js new file mode 100644 index 000000000000000..794cf77cae22f8b --- /dev/null +++ b/test/integration/mixed-ssg-serverprops-error/test/index.test.js @@ -0,0 +1,32 @@ +/* eslint-env jest */ +/* global jasmine */ +import fs from 'fs-extra' +import { join } from 'path' +import { nextBuild } from 'next-test-utils' +import { SERVER_PROPS_SSG_CONFLICT } from 'next/dist/lib/constants' + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1 +const appDir = join(__dirname, '..') +const indexPage = join(appDir, 'pages/index.js') +const indexPageAlt = `${indexPage}.alt` +const indexPageBak = `${indexPage}.bak` + +describe('Mixed getStaticProps and getServerProps error', () => { + it('should error when exporting both getStaticProps and getServerProps', async () => { + const { stderr } = await nextBuild(appDir, [], { stderr: true }) + expect(stderr).toContain(SERVER_PROPS_SSG_CONFLICT) + }) + + it('should error when exporting both getStaticPaths and getServerProps', async () => { + await fs.move(indexPage, indexPageBak) + await fs.move(indexPageAlt, indexPage) + + const { stderr, code } = await nextBuild(appDir, [], { stderr: true }) + + await fs.move(indexPage, indexPageAlt) + await fs.move(indexPageBak, indexPage) + + expect(code).toBe(1) + expect(stderr).toContain(SERVER_PROPS_SSG_CONFLICT) + }) +}) From ff69a10ff363fff3cf74d51908165b032a62bad8 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 16:42:53 -0600 Subject: [PATCH 13/23] Add comment explaining params chosing in serverless-loader --- packages/next/build/webpack/loaders/next-serverless-loader.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 27fbedd28b4111c..ce8b3f5d2ad82ad 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -274,6 +274,8 @@ const nextServerlessLoader: loader.Loader = function() { ` : `const nowParams = null;` } + // make sure to set renderOpts to the correct params e.g. _params + // if provided from worker or params if we're parsing them here renderOpts.params = _params || params let result = await renderToHTML(req, res, "${page}", Object.assign({}, unstable_getStaticProps ? {} : parsedUrl.query, nowParams ? nowParams : params, _params), renderOpts) From 862a61786b410e5abc7c1584fd4e6fb0af16b627 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 16:45:26 -0600 Subject: [PATCH 14/23] Update invalidKeysMsg to return a string and inline throwing --- packages/next/next-server/server/render.tsx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 041959cd7da1bcd..a713eff150d206a 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -262,6 +262,14 @@ function renderDocument( ) } +const invalidKeysMsg = (methodName: string, invalidKeys: string[]) => { + return ( + `Additional keys were returned from \`${methodName}\`. Properties intended for your component must be nested under the \`props\` key, e.g.:` + + `\n\n\treturn { props: { title: 'My Title', content: '...' } }` + + `\n\nKeys that need to be moved: ${invalidKeys.join(', ')}.` + ) +} + export async function renderToHTML( req: IncomingMessage, res: ServerResponse, @@ -439,14 +447,6 @@ export async function renderToHTML( ) - const invalidKeysMsg = (methodName: string, invalidKeys: string[]) => { - throw new Error( - `Additional keys were returned from \`${methodName}\`. Properties intended for your component must be nested under the \`props\` key, e.g.:` + - `\n\n\treturn { props: { title: 'My Title', content: '...' } }` + - `\n\nKeys that need to be moved: ${invalidKeys.join(', ')}.` - ) - } - try { props = await loadGetInitialProps(App, { AppTree: ctx.AppTree, @@ -465,7 +465,7 @@ export async function renderToHTML( ) if (invalidKeys.length) { - invalidKeysMsg('getStaticProps', invalidKeys) + throw new Error(invalidKeysMsg('getStaticProps', invalidKeys)) } if (typeof data.revalidate === 'number') { @@ -521,7 +521,7 @@ export async function renderToHTML( const invalidKeys = Object.keys(data).filter(key => key !== 'props') if (invalidKeys.length) { - invalidKeysMsg('getServerProps', invalidKeys) + throw new Error(invalidKeysMsg('getServerProps', invalidKeys)) } props.pageProps = data.props From 9a2a4def883268e3d9d7d8a9556f45e82e9b1617 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 16:52:10 -0600 Subject: [PATCH 15/23] Inline throwing mixed SSG/serverProps error --- .../next/build/babel/plugins/next-ssg-transform.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/next/build/babel/plugins/next-ssg-transform.ts b/packages/next/build/babel/plugins/next-ssg-transform.ts index a93b2a9e26372af..19c5699db61fa0e 100644 --- a/packages/next/build/babel/plugins/next-ssg-transform.ts +++ b/packages/next/build/babel/plugins/next-ssg-transform.ts @@ -60,17 +60,17 @@ function decorateSsgExport( }) } -const mixedSsgServerPropsError = () => { - throw new Error(SERVER_PROPS_SSG_CONFLICT) -} - const checkIsSSG = (name: string, state: PluginState, toRemove: any) => { if (ssgExports.has(name)) { if (name === EXPORT_NAME_GET_SERVER_PROPS) { - if (state.isPrerender) mixedSsgServerPropsError() + if (state.isPrerender) { + throw new Error(SERVER_PROPS_SSG_CONFLICT) + } state.isServerProps = true } else { - if (state.isServerProps) mixedSsgServerPropsError() + if (state.isServerProps) { + throw new Error(SERVER_PROPS_SSG_CONFLICT) + } state.isPrerender = true } toRemove.remove() From a3c257678f33d57f7aa4d6c395c6a3fa35a9a17e Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 16:55:51 -0600 Subject: [PATCH 16/23] Update setting cache header in serverless-loader --- .../webpack/loaders/next-serverless-loader.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index ce8b3f5d2ad82ad..5b0a388e9b78c1c 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -285,14 +285,12 @@ const nextServerlessLoader: loader.Loader = function() { res.setHeader('Content-Type', 'application/json') res.setHeader('Content-Length', Buffer.byteLength(payload)) - if (renderOpts.revalidate) { - res.setHeader( - 'Cache-Control', - renderOpts.revalidate < 0 - ? \`no-cache, no-store, must-revalidate\` - : \`s-maxage=\${renderOpts.revalidate}, stale-while-revalidate\` - ) - } + res.setHeader( + 'Cache-Control', + unstable_getServerProps + ? \`no-cache, no-store, must-revalidate\` + : \`s-maxage=\${renderOpts.revalidate}, stale-while-revalidate\` + ) res.end(payload) return null } From bb290f1c85d2d2f3b3776fedc06eb62d15c6b5c7 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 24 Jan 2020 17:25:03 -0600 Subject: [PATCH 17/23] Add separate getServerData method in router --- .../next/next-server/lib/router/router.ts | 96 +++++++++++-------- 1 file changed, 55 insertions(+), 41 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index a0355e6e7e0b4eb..7c110f30eaa2a01 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -27,6 +27,9 @@ function toRoute(path: string): string { return path.replace(/\/$/, '') || '/' } +const prepareRoute = (path: string) => + toRoute(!path || path === '/' ? '/index' : path) + type Url = UrlObject | string export type BaseRouter = { @@ -61,6 +64,33 @@ type BeforePopStateCallback = (state: any) => boolean type ComponentLoadCancel = (() => void) | null +const fetchNextData = ( + pathname: string, + query: ParsedUrlQuery | null, + cb?: (...args: any) => any +) => { + return fetch( + formatWithValidation({ + // @ts-ignore __NEXT_DATA__ + pathname: `/_next/data/${__NEXT_DATA__.buildId}${pathname}.json`, + query, + }) + ) + .then(res => { + if (!res.ok) { + throw new Error(`Failed to load static props`) + } + return res.json() + }) + .then(data => { + return cb ? cb(data) : data + }) + .catch((err: Error) => { + ;(err as any).code = 'PAGE_LOAD_ERROR' + throw err + }) +} + export default class Router implements BaseRouter { route: string pathname: string @@ -461,21 +491,22 @@ export default class Router implements BaseRouter { } } - const isServerProps = (Component as any).__N_SSP - - return this._getData(() => - (Component as any).__N_SSG || isServerProps - ? this._getStaticData(as, !isServerProps) - : this.getInitialProps( - Component, - // we provide AppTree later so this needs to be `any` - { - pathname, - query, - asPath: as, - } as any - ) - ).then(props => { + return this._getData(() => { + if ((Component as any).__N_SSG) { + return this._getStaticData(as) + } else if ((Component as any).__N_SSP) { + return this._getServerData(as) + } + return this.getInitialProps( + Component, + // we provide AppTree later so this needs to be `any` + { + pathname, + query, + asPath: as, + } as any + ) + }).then(props => { routeInfo.props = props this.components[route] = routeInfo return routeInfo @@ -678,35 +709,18 @@ export default class Router implements BaseRouter { }) } - _getStaticData = (asPath: string, cache?: boolean): Promise => { - let { pathname, query } = parse(asPath, true) - pathname = toRoute(!pathname || pathname === '/' ? '/index' : pathname) + _getStaticData = (asPath: string): Promise => { + const pathname = prepareRoute(parse(asPath).pathname!) return process.env.NODE_ENV === 'production' && this.sdc[pathname] ? Promise.resolve(this.sdc[pathname]) - : fetch( - formatWithValidation({ - // @ts-ignore __NEXT_DATA__ - pathname: `/_next/data/${__NEXT_DATA__.buildId}${pathname}.json`, - query, - }) - ) - .then(res => { - if (!res.ok) { - throw new Error(`Failed to load static props`) - } - return res.json() - }) - .then(data => { - if (cache) { - this.sdc[pathname!] = data - } - return data - }) - .catch((err: Error) => { - ;(err as any).code = 'PAGE_LOAD_ERROR' - throw err - }) + : fetchNextData(pathname, null, data => (this.sdc[pathname] = data)) + } + + _getServerData = (asPath: string): Promise => { + let { pathname, query } = parse(asPath, true) + pathname = prepareRoute(pathname!) + return fetchNextData(pathname, query) } getInitialProps( From 15bf4076858d7ccb4572ded140a200ddec2fa15c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jan 2020 15:20:58 -0600 Subject: [PATCH 18/23] Update checkIsSSG -> isDataIdentifier --- .../build/babel/plugins/next-ssg-transform.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/next/build/babel/plugins/next-ssg-transform.ts b/packages/next/build/babel/plugins/next-ssg-transform.ts index 19c5699db61fa0e..8fc50b255a7a9b3 100644 --- a/packages/next/build/babel/plugins/next-ssg-transform.ts +++ b/packages/next/build/babel/plugins/next-ssg-transform.ts @@ -60,7 +60,7 @@ function decorateSsgExport( }) } -const checkIsSSG = (name: string, state: PluginState, toRemove: any) => { +const isDataIdentifier = (name: string, state: PluginState): boolean => { if (ssgExports.has(name)) { if (name === EXPORT_NAME_GET_SERVER_PROPS) { if (state.isPrerender) { @@ -73,8 +73,9 @@ const checkIsSSG = (name: string, state: PluginState, toRemove: any) => { } state.isPrerender = true } - toRemove.remove() + return true } + return false } export default function nextTransformSsg({ @@ -262,7 +263,9 @@ export default function nextTransformSsg({ const specifiers = path.get('specifiers') if (specifiers.length) { specifiers.forEach(s => { - checkIsSSG(s.node.exported.name, state, s) + if (isDataIdentifier(s.node.exported.name, state)) { + s.remove() + } }) if (path.node.specifiers.length < 1) { @@ -279,7 +282,9 @@ export default function nextTransformSsg({ switch (decl.node.type) { case 'FunctionDeclaration': { const name = decl.node.id!.name - checkIsSSG(name, state, path) + if (isDataIdentifier(name, state)) { + path.remove() + } break } case 'VariableDeclaration': { @@ -291,7 +296,9 @@ export default function nextTransformSsg({ return } const name = d.node.id.name - checkIsSSG(name, state, d) + if (isDataIdentifier(name, state)) { + d.remove() + } }) break } From 1d8752be694b6ed80819bae3ca6c3089fb781823 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jan 2020 15:23:45 -0600 Subject: [PATCH 19/23] Refactor router getData back to ternary --- .../next/next-server/lib/router/router.ts | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 7c110f30eaa2a01..93421e14c06d4b5 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -492,20 +492,19 @@ export default class Router implements BaseRouter { } return this._getData(() => { - if ((Component as any).__N_SSG) { - return this._getStaticData(as) - } else if ((Component as any).__N_SSP) { - return this._getServerData(as) - } - return this.getInitialProps( - Component, - // we provide AppTree later so this needs to be `any` - { - pathname, - query, - asPath: as, - } as any - ) + return (Component as any).__N_SSG + ? this._getStaticData(as) + : (Component as any).__N_SSP + ? this._getServerData(as) + : this.getInitialProps( + Component, + // we provide AppTree later so this needs to be `any` + { + pathname, + query, + asPath: as, + } as any + ) }).then(props => { routeInfo.props = props this.components[route] = routeInfo From 27bbb2b8b4254703d3bfb37fe1dd8aa1f93f437e Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jan 2020 15:26:54 -0600 Subject: [PATCH 20/23] Apply suggestions to build/index.ts --- packages/next/build/index.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 76aa5f9e9a90e8d..67cbdc381bf684e 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -63,6 +63,7 @@ import { } from './utils' import getBaseWebpackConfig from './webpack-config' import { writeBuildId } from './write-build-id' +import escapeStringRegexp from 'escape-string-regexp' const fsAccess = promisify(fs.access) const fsUnlink = promisify(fs.unlink) @@ -535,11 +536,6 @@ export default async function build(dir: string, conf = null): Promise { routesManifest.serverPropsRoutes = {} for (const page of serverPropsPages) { - const cleanDataRoute = path.posix.join( - '/_next/data', - buildId.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'), - `${page === '/' ? '/index' : page}.json` - ) const dataRoute = path.posix.join( '/_next/data', buildId, @@ -553,7 +549,13 @@ export default async function build(dir: string, conf = null): Promise { /\(\?:\\\/\)\?\$$/, '\\.json$' ) - : new RegExp(`^${cleanDataRoute}$`).source, + : new RegExp( + `^${path.posix.join( + '/_next/data', + escapeStringRegexp(buildId), + `${page === '/' ? '/index' : page}.json` + )}$` + ).source, } } From d9ee71c4fa3d358a8fd546a15c6f8863f46876af Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jan 2020 16:29:28 -0600 Subject: [PATCH 21/23] drop return --- packages/next/next-server/lib/router/router.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 93421e14c06d4b5..4c5a30545b8399b 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -491,8 +491,8 @@ export default class Router implements BaseRouter { } } - return this._getData(() => { - return (Component as any).__N_SSG + return this._getData(() => + (Component as any).__N_SSG ? this._getStaticData(as) : (Component as any).__N_SSP ? this._getServerData(as) @@ -505,7 +505,7 @@ export default class Router implements BaseRouter { asPath: as, } as any ) - }).then(props => { + ).then(props => { routeInfo.props = props this.components[route] = routeInfo return routeInfo From 2e8ef843971878c10c2f99232763a1bafa8e12fc Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jan 2020 16:36:14 -0600 Subject: [PATCH 22/23] De-dupe extra escape regex --- test/integration/getserverprops/test/index.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index 992d1d0b75fe660..4c3dc963f4ee9dc 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -4,6 +4,7 @@ import fs from 'fs-extra' import { join } from 'path' import webdriver from 'next-webdriver' import cheerio from 'cheerio' +import escapeRegex from 'escape-string-regexp' import { renderViaHTTP, fetchViaHTTP, @@ -23,8 +24,6 @@ let app let appPort let buildId -const escapeRegex = str => str.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&') - const expectedManifestRoutes = () => ({ '/something': { page: '/something', From 4fe847aa4a7cf538c4e25260086a6b28bcaf0a2c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jan 2020 16:39:54 -0600 Subject: [PATCH 23/23] Add param test --- test/integration/getserverprops/test/index.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/integration/getserverprops/test/index.test.js b/test/integration/getserverprops/test/index.test.js index 4c3dc963f4ee9dc..1c932da181e2a68 100644 --- a/test/integration/getserverprops/test/index.test.js +++ b/test/integration/getserverprops/test/index.test.js @@ -249,6 +249,16 @@ const runTests = (dev = false) => { expect(text).toMatch(/a normal page/) }) + it('should provide correct query value for dynamic page', async () => { + const html = await renderViaHTTP( + appPort, + '/blog/post-1?post=something-else' + ) + const $ = cheerio.load(html) + const query = JSON.parse($('#query').text()) + expect(query.post).toBe('post-1') + }) + it('should parse query values on mount correctly', async () => { const browser = await webdriver(appPort, '/blog/post-1?another=value') await waitFor(2000)