Fix AMP HMR for child components (#9736)

vercel · Dec 16, 2019 · fd95d6c · fd95d6c
1 parent 87d9b2e
commit fd95d6c
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 11 deletions.
diff --git a/packages/next/build/babel/plugins/next-page-config.ts b/packages/next/build/babel/plugins/next-page-config.ts
@@ -81,7 +81,17 @@ export default function nextPageConfig({
                 }
 
                 if (config.amp === true) {
-                  replaceBundle(path, t)
+                  if (
+                    !(
+                      state.file &&
+                      state.file.opts &&
+                      state.file.opts.caller.isDev
+                    )
+                  ) {
+                    // don't replace bundle in development so HMR can track
+                    // dependencies and trigger reload when they are changed
+                    replaceBundle(path, t)
+                  }
                   state.bundleDropped = true
                   return
                 }

diff --git a/packages/next/build/webpack/loaders/next-babel-loader.js b/packages/next/build/webpack/loaders/next-babel-loader.js
@@ -130,6 +130,7 @@ module.exports = babelLoader.custom(babel => {
 
       options.caller.isServer = isServer
       options.caller.isModern = isModern
+      options.caller.isDev = development
 
       options.plugins = options.plugins || []
 

diff --git a/packages/next/server/hot-reloader.ts b/packages/next/server/hot-reloader.ts
@@ -1,7 +1,5 @@
-import fs from 'fs'
 import { IncomingMessage, ServerResponse } from 'http'
 import { join, normalize, relative as relativePath, sep } from 'path'
-import { promisify } from 'util'
 import webpack from 'webpack'
 import WebpackDevMiddleware from 'webpack-dev-middleware'
 import WebpackHotMiddleware from 'webpack-hot-middleware'
@@ -25,9 +23,6 @@ import onDemandEntryHandler, { normalizePage } from './on-demand-entry-handler'
 import { NextHandleFunction } from 'connect'
 import { UrlObject } from 'url'
 
-const access = promisify(fs.access)
-const readFile = promisify(fs.readFile)
-
 export async function renderScriptError(res: ServerResponse, error: Error) {
   // Asks CDNs and others to not to cache the errored page
   res.setHeader(
@@ -196,15 +191,14 @@ export default class HotReloader {
         const bundlePath = join(
           this.dir,
           this.config.distDir,
-          'static/development/pages',
+          'server/static/development/pages',
           page + '.js'
         )
 
-        // make sure to 404 for AMP bundles in case they weren't removed
+        // Make sure to 404 for AMP first pages
         try {
-          await access(bundlePath)
-          const data = await readFile(bundlePath, 'utf8')
-          if (data.includes('__NEXT_DROP_CLIENT_FILE__')) {
+          const mod = require(bundlePath)
+          if (mod && mod.config && mod.config.amp === true) {
             res.statusCode = 404
             res.end()
             return { finished: true }

diff --git a/test/integration/amphtml/components/hello.js b/test/integration/amphtml/components/hello.js
@@ -0,0 +1 @@
+export default () => <p id="hello-comp">hello</p>
diff --git a/test/integration/amphtml/pages/hmr/comp.js b/test/integration/amphtml/pages/hmr/comp.js
@@ -0,0 +1,10 @@
+import Hello from '../../components/hello'
+
+export const config = { amp: true }
+
+export default () => (
+  <>
+    <Hello />
+    <span>{new Date().getTime()}</span>
+  </>
+)
diff --git a/test/integration/amphtml/test/index.test.js b/test/integration/amphtml/test/index.test.js
@@ -356,6 +356,23 @@ describe('AMP Usage', () => {
       }
     })
 
+    it('should detect changes to component and refresh an AMP page', async () => {
+      const browser = await webdriver(dynamicAppPort, '/hmr/comp')
+      const text = await browser.elementByCss('#hello-comp').text()
+      expect(text).toBe('hello')
+
+      const testComp = join(__dirname, '../components/hello.js')
+
+      const origContent = readFileSync(testComp, 'utf8')
+      const newContent = origContent.replace('>hello<', '>hi<')
+
+      writeFileSync(testComp, newContent, 'utf8')
+      await check(() => browser.elementByCss('#hello-comp').text(), /hi/)
+
+      writeFileSync(testComp, origContent, 'utf8')
+      await check(() => browser.elementByCss('#hello-comp').text(), /hello/)
+    })
+
     it('should not reload unless the page is edited for an AMP page', async () => {
       let browser
       try {