From 260707de68f36c2964ae5f8df9b0caf6e0522381 Mon Sep 17 00:00:00 2001
From: Tim Neutkens <tim@timneutkens.nl>
Date: Wed, 1 Jul 2020 16:59:18 +0200
Subject: [PATCH] Add etag support for getServerSideProps/getStaticProps pages
 (#14760)

Fixes #11711

Also cleaned up some extra code.

This was already supported on the Vercel edge network.
---
 .../webpack/loaders/next-serverless-loader.ts |   8 +-
 .../next/next-server/server/next-server.ts    | 128 ++++++++----------
 .../next/next-server/server/send-payload.ts   |  19 ++-
 packages/next/server/next-dev-server.ts       |  28 ++++
 .../production/pages/fully-dynamic.js         |  11 ++
 .../production/pages/fully-static.js          |  11 ++
 .../integration/production/test/index.test.js |  18 +++
 7 files changed, 147 insertions(+), 76 deletions(-)
 create mode 100644 test/integration/production/pages/fully-dynamic.js
 create mode 100644 test/integration/production/pages/fully-static.js

diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts
index 3cc4d959dc..7995b67729 100644
--- a/packages/next/build/webpack/loaders/next-serverless-loader.ts
+++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts
@@ -411,7 +411,9 @@ const nextServerlessLoader: loader.Loader = function () {
 
         if (!renderMode) {
           if (_nextData || getStaticProps || getServerSideProps) {
-            sendPayload(res, _nextData ? JSON.stringify(renderOpts.pageData) : result, _nextData ? 'json' : 'html', {
+            sendPayload(req, res, _nextData ? JSON.stringify(renderOpts.pageData) : result, _nextData ? 'json' : 'html', ${
+              generateEtags === 'true' ? true : false
+            }, {
               private: isPreviewMode,
               stateful: !!getServerSideProps,
               revalidate: renderOpts.revalidate,
@@ -482,7 +484,9 @@ const nextServerlessLoader: loader.Loader = function () {
         await initServer()
         const html = await renderReqToHTML(req, res)
         if (html) {
-          sendHTML(req, res, html, {generateEtags: ${generateEtags}})
+          sendHTML(req, res, html, {generateEtags: ${
+            generateEtags === 'true' ? true : false
+          }})
         }
       } catch(err) {
         console.error(err)
diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts
index 608af3f35c..8633120cae 100644
--- a/packages/next/next-server/server/next-server.ts
+++ b/packages/next/next-server/server/next-server.ts
@@ -127,9 +127,6 @@ export default class Server {
   router: Router
   protected dynamicRoutes?: DynamicRoutes
   protected customRoutes: CustomRoutes
-  protected staticPathsWorker?: import('jest-worker').default & {
-    loadStaticPaths: typeof import('../../server/static-paths-worker').loadStaticPaths
-  }
 
   public constructor({
     dir = '.',
@@ -885,42 +882,20 @@ export default class Server {
     return null
   }
 
-  private async getStaticPaths(
+  protected async getStaticPaths(
     pathname: string
   ): Promise<{
     staticPaths: string[] | undefined
     hasStaticFallback: boolean
   }> {
-    // we lazy load the staticPaths to prevent the user
-    // from waiting on them for the page to load in dev mode
-    let staticPaths: string[] | undefined
-    let hasStaticFallback = false
-
-    if (!this.renderOpts.dev) {
-      // `staticPaths` is intentionally set to `undefined` as it should've
-      // been caught when checking disk data.
-      staticPaths = undefined
-
-      // Read whether or not fallback should exist from the manifest.
-      hasStaticFallback =
-        typeof this.getPrerenderManifest().dynamicRoutes[pathname].fallback ===
-        'string'
-    } else {
-      const __getStaticPaths = async () => {
-        const paths = await this.staticPathsWorker!.loadStaticPaths(
-          this.distDir,
-          pathname,
-          !this.renderOpts.dev && this._isLikeServerless
-        )
-        return paths
-      }
-      ;({ paths: staticPaths, fallback: hasStaticFallback } = (
-        await withCoalescedInvoke(__getStaticPaths)(
-          `staticPaths-${pathname}`,
-          []
-        )
-      ).value)
-    }
+    // `staticPaths` is intentionally set to `undefined` as it should've
+    // been caught when checking disk data.
+    const staticPaths = undefined
+
+    // Read whether or not fallback should exist from the manifest.
+    const hasStaticFallback =
+      typeof this.getPrerenderManifest().dynamicRoutes[pathname].fallback ===
+      'string'
 
     return { staticPaths, hasStaticFallback }
   }
@@ -1000,9 +975,11 @@ export default class Server {
         : cachedData.html
 
       sendPayload(
+        req,
         res,
         data,
         isDataReq ? 'json' : 'html',
+        this.renderOpts.generateEtags,
         !this.renderOpts.dev
           ? {
               private: isPreviewMode,
@@ -1029,43 +1006,51 @@ export default class Server {
           return { isOrigin: true, value }
         }
 
-    const doRender = maybeCoalesceInvoke(async function (): Promise<{
-      html: string | null
-      pageData: any
-      sprRevalidate: number | false
-    }> {
-      let pageData: any
-      let html: string | null
-      let sprRevalidate: number | false
-
-      let renderResult
-      // handle serverless
-      if (isLikeServerless) {
-        renderResult = await (components.Component as any).renderReqToHTML(
-          req,
-          res,
-          'passthrough'
-        )
+    const doRender = maybeCoalesceInvoke(
+      async (): Promise<{
+        html: string | null
+        pageData: any
+        sprRevalidate: number | false
+      }> => {
+        let pageData: any
+        let html: string | null
+        let sprRevalidate: number | false
+
+        let renderResult
+        // handle serverless
+        if (isLikeServerless) {
+          renderResult = await (components.Component as any).renderReqToHTML(
+            req,
+            res,
+            'passthrough'
+          )
 
-        html = renderResult.html
-        pageData = renderResult.renderOpts.pageData
-        sprRevalidate = renderResult.renderOpts.revalidate
-      } else {
-        const renderOpts: RenderOpts = {
-          ...components,
-          ...opts,
-          isDataReq,
+          html = renderResult.html
+          pageData = renderResult.renderOpts.pageData
+          sprRevalidate = renderResult.renderOpts.revalidate
+        } else {
+          const renderOpts: RenderOpts = {
+            ...components,
+            ...opts,
+            isDataReq,
+          }
+          renderResult = await renderToHTML(
+            req,
+            res,
+            pathname,
+            query,
+            renderOpts
+          )
+
+          html = renderResult
+          // TODO: change this to a different passing mechanism
+          pageData = (renderOpts as any).pageData
+          sprRevalidate = (renderOpts as any).revalidate
         }
-        renderResult = await renderToHTML(req, res, pathname, query, renderOpts)
 
-        html = renderResult
-        // TODO: change this to a different passing mechanism
-        pageData = (renderOpts as any).pageData
-        sprRevalidate = (renderOpts as any).revalidate
+        return { html, pageData, sprRevalidate }
       }
-
-      return { html, pageData, sprRevalidate }
-    })
+    )
 
     const isProduction = !this.renderOpts.dev
     const isDynamicPathname = isDynamicRoute(pathname)
@@ -1075,9 +1060,6 @@ export default class Server {
       ? await this.getStaticPaths(pathname)
       : { staticPaths: undefined, hasStaticFallback: false }
 
-    // const isForcedBlocking =
-    //   req.headers['X-Prerender-Bypass-Mode'] !== 'Blocking'
-
     // When we did not respond from cache, we need to choose to block on
     // rendering or return a skeleton.
     //
@@ -1127,7 +1109,7 @@ export default class Server {
         html = renderResult.html
       }
 
-      sendPayload(res, html, 'html')
+      sendPayload(req, res, html, 'html', this.renderOpts.generateEtags)
       return null
     }
 
@@ -1138,9 +1120,11 @@ export default class Server {
     let resHtml = html
     if (!isResSent(res) && (isSSG || isDataReq || isServerProps)) {
       sendPayload(
+        req,
         res,
         isDataReq ? JSON.stringify(pageData) : html,
         isDataReq ? 'json' : 'html',
+        this.renderOpts.generateEtags,
         !this.renderOpts.dev || (isServerProps && !isDataReq)
           ? {
               private: isPreviewMode,
@@ -1441,7 +1425,7 @@ export default class Server {
     }
   }
 
-  private get _isLikeServerless(): boolean {
+  protected get _isLikeServerless(): boolean {
     return isTargetLikeServerless(this.nextConfig.target)
   }
 }
diff --git a/packages/next/next-server/server/send-payload.ts b/packages/next/next-server/server/send-payload.ts
index 0a222ebdbb..e6f2b5a005 100644
--- a/packages/next/next-server/server/send-payload.ts
+++ b/packages/next/next-server/server/send-payload.ts
@@ -1,10 +1,14 @@
-import { ServerResponse } from 'http'
+import { IncomingMessage, ServerResponse } from 'http'
 import { isResSent } from '../lib/utils'
+import generateETag from 'next/dist/compiled/etag'
+import fresh from 'next/dist/compiled/fresh'
 
 export function sendPayload(
+  req: IncomingMessage,
   res: ServerResponse,
   payload: any,
   type: 'html' | 'json',
+  generateEtags: boolean,
   options?:
     | { private: true }
     | { private: boolean; stateful: true }
@@ -14,7 +18,18 @@ export function sendPayload(
     return
   }
 
-  // TODO: ETag headers?
+  const etag = generateEtags ? generateETag(payload) : undefined
+
+  if (fresh(req.headers, { etag })) {
+    res.statusCode = 304
+    res.end()
+    return
+  }
+
+  if (etag) {
+    res.setHeader('ETag', etag)
+  }
+
   res.setHeader(
     'Content-Type',
     type === 'json' ? 'application/json' : 'text/html; charset=utf-8'
diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts
index aea4c2c7f3..cd89e573d2 100644
--- a/packages/next/server/next-dev-server.ts
+++ b/packages/next/server/next-dev-server.ts
@@ -32,6 +32,7 @@ import { Telemetry } from '../telemetry/storage'
 import HotReloader from './hot-reloader'
 import { findPageFile } from './lib/find-page-file'
 import { getNodeOptionsWithoutInspect } from './lib/utils'
+import { withCoalescedInvoke } from '../lib/coalesced-function'
 
 if (typeof React.Suspense === 'undefined') {
   throw new Error(
@@ -45,6 +46,9 @@ export default class DevServer extends Server {
   private webpackWatcher?: Watchpack | null
   private hotReloader?: HotReloader
   private isCustomServer: boolean
+  protected staticPathsWorker: import('jest-worker').default & {
+    loadStaticPaths: typeof import('./static-paths-worker').loadStaticPaths
+  }
 
   constructor(options: ServerConstructor & { isNextDevCommand?: boolean }) {
     super({ ...options, dev: true })
@@ -446,6 +450,30 @@ export default class DevServer extends Server {
     return !snippet.includes('data-amp-development-mode-only')
   }
 
+  protected async getStaticPaths(
+    pathname: string
+  ): Promise<{
+    staticPaths: string[] | undefined
+    hasStaticFallback: boolean
+  }> {
+    // we lazy load the staticPaths to prevent the user
+    // from waiting on them for the page to load in dev mode
+
+    const __getStaticPaths = async () => {
+      const paths = await this.staticPathsWorker.loadStaticPaths(
+        this.distDir,
+        pathname,
+        !this.renderOpts.dev && this._isLikeServerless
+      )
+      return paths
+    }
+    const { paths: staticPaths, fallback: hasStaticFallback } = (
+      await withCoalescedInvoke(__getStaticPaths)(`staticPaths-${pathname}`, [])
+    ).value
+
+    return { staticPaths, hasStaticFallback }
+  }
+
   protected async ensureApiPage(pathname: string) {
     return this.hotReloader!.ensurePage(pathname)
   }
diff --git a/test/integration/production/pages/fully-dynamic.js b/test/integration/production/pages/fully-dynamic.js
new file mode 100644
index 0000000000..18e66fb035
--- /dev/null
+++ b/test/integration/production/pages/fully-dynamic.js
@@ -0,0 +1,11 @@
+export async function getServerSideProps() {
+  return {
+    props: {
+      myDynamicProp: 'hello world',
+    },
+  }
+}
+
+export default function FullyDynamic({ myDynamicProp }) {
+  return <h1>{myDynamicProp}</h1>
+}
diff --git a/test/integration/production/pages/fully-static.js b/test/integration/production/pages/fully-static.js
new file mode 100644
index 0000000000..4e43dc8661
--- /dev/null
+++ b/test/integration/production/pages/fully-static.js
@@ -0,0 +1,11 @@
+export async function getStaticProps() {
+  return {
+    props: {
+      myStaticProp: 'hello world',
+    },
+  }
+}
+
+export default function FullyStatic({ myStaticProp }) {
+  return <h1>{myStaticProp}</h1>
+}
diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js
index 263a53929c..7201d9f0bb 100644
--- a/test/integration/production/test/index.test.js
+++ b/test/integration/production/test/index.test.js
@@ -79,6 +79,24 @@ describe('Production Usage', () => {
       expect(res2.status).toBe(304)
     })
 
+    it('should allow etag header support with getStaticProps', async () => {
+      const url = `http://localhost:${appPort}/fully-static`
+      const etag = (await fetch(url)).headers.get('ETag')
+
+      const headers = { 'If-None-Match': etag }
+      const res2 = await fetch(url, { headers })
+      expect(res2.status).toBe(304)
+    })
+
+    it('should allow etag header support with getServerSideProps', async () => {
+      const url = `http://localhost:${appPort}/fully-dynamic`
+      const etag = (await fetch(url)).headers.get('ETag')
+
+      const headers = { 'If-None-Match': etag }
+      const res2 = await fetch(url, { headers })
+      expect(res2.status).toBe(304)
+    })
+
     it('should have X-Powered-By header support', async () => {
       const url = `http://localhost:${appPort}/`
       const header = (await fetch(url)).headers.get('X-Powered-By')
-- 
GitLab