From dd9811b206bb554251b34f4daebb052eb67f9b69 Mon Sep 17 00:00:00 2001
From: Tim Neutkens <tim@timneutkens.nl>
Date: Sun, 17 Feb 2019 19:52:00 +0100
Subject: [PATCH] Fix recursive hydration of next/dynamic (#6326)

Fixes #5347

The main issue is that we were waiting only 1 level of dynamic imports, so the dynamic imports nested inside other dynamic import files were not awaited. This would cause either a flash of loading states or you wouldn't see the loading state (because of preload) but it would then show a hydration warning in development.

Thanks to @arthens for providing the reproduction that I modelled the tests after.
---
 packages/next-server/lib/loadable.js          | 44 ++++++++-----------
 test/integration/basic/components/nested1.js  |  8 ++++
 test/integration/basic/components/nested2.js  | 12 +++++
 .../integration/basic/pages/dynamic/nested.js |  5 +++
 test/integration/basic/test/dynamic.js        | 20 +++++++++
 5 files changed, 63 insertions(+), 26 deletions(-)
 create mode 100644 test/integration/basic/components/nested1.js
 create mode 100644 test/integration/basic/components/nested2.js
 create mode 100644 test/integration/basic/pages/dynamic/nested.js

diff --git a/packages/next-server/lib/loadable.js b/packages/next-server/lib/loadable.js
index 635e04de..9b352c7c 100644
--- a/packages/next-server/lib/loadable.js
+++ b/packages/next-server/lib/loadable.js
@@ -25,7 +25,7 @@ import React from 'react'
 import PropTypes from 'prop-types'
 
 const ALL_INITIALIZERS = []
-const READY_INITIALIZERS = new Map()
+const READY_INITIALIZERS = []
 let initialized = false
 
 function load (loader) {
@@ -138,11 +138,13 @@ function createLoadableComponent (loadFn, options) {
   // Client only
   if (!initialized && typeof window !== 'undefined' && typeof opts.webpack === 'function') {
     const moduleIds = opts.webpack()
-    for (const moduleId of moduleIds) {
-      READY_INITIALIZERS.set(moduleId, () => {
-        return init()
-      })
-    }
+    READY_INITIALIZERS.push((ids) => {
+      for (const moduleId of moduleIds) {
+        if (ids.indexOf(moduleId) !== -1) {
+          return init()
+        }
+      }
+    })
   }
 
   return class LoadableComponent extends React.Component {
@@ -273,17 +275,17 @@ function LoadableMap (opts) {
 
 Loadable.Map = LoadableMap
 
-function flushInitializers (initializers) {
+function flushInitializers (initializers, ids) {
   let promises = []
 
   while (initializers.length) {
     let init = initializers.pop()
-    promises.push(init())
+    promises.push(init(ids))
   }
 
   return Promise.all(promises).then(() => {
     if (initializers.length) {
-      return flushInitializers(initializers)
+      return flushInitializers(initializers, ids)
     }
   })
 }
@@ -294,24 +296,14 @@ Loadable.preloadAll = () => {
   })
 }
 
-Loadable.preloadReady = (webpackIds) => {
-  return new Promise((resolve, reject) => {
-    const initializers = webpackIds.reduce((allInitalizers, moduleId) => {
-      const initializer = READY_INITIALIZERS.get(moduleId)
-      if (!initializer) {
-        return allInitalizers
-      }
-
-      allInitalizers.push(initializer)
-      return allInitalizers
-    }, [])
-
-    initialized = true
-    // Make sure the object is cleared
-    READY_INITIALIZERS.clear()
-
+Loadable.preloadReady = (ids) => {
+  return new Promise((resolve) => {
+    const res = () => {
+      initialized = true
+      return resolve()
+    }
     // We always will resolve, errors should be handled within loading UIs.
-    flushInitializers(initializers).then(resolve, resolve)
+    flushInitializers(READY_INITIALIZERS, ids).then(res, res)
   })
 }
 
diff --git a/test/integration/basic/components/nested1.js b/test/integration/basic/components/nested1.js
new file mode 100644
index 00000000..a3d45491
--- /dev/null
+++ b/test/integration/basic/components/nested1.js
@@ -0,0 +1,8 @@
+import dynamic from 'next/dynamic'
+
+const Nested2 = dynamic(() => import('./nested2'))
+
+export default () => <div>
+  Nested 1
+  <Nested2 />
+</div>
diff --git a/test/integration/basic/components/nested2.js b/test/integration/basic/components/nested2.js
new file mode 100644
index 00000000..da4a3108
--- /dev/null
+++ b/test/integration/basic/components/nested2.js
@@ -0,0 +1,12 @@
+import dynamic from 'next/dynamic'
+
+const BrowserLoaded = dynamic(async () => () => <div>Browser hydrated</div>, {
+  ssr: false
+})
+
+export default () => <div>
+  <div>
+    Nested 2
+  </div>
+  <BrowserLoaded />
+</div>
diff --git a/test/integration/basic/pages/dynamic/nested.js b/test/integration/basic/pages/dynamic/nested.js
new file mode 100644
index 00000000..053e6b1d
--- /dev/null
+++ b/test/integration/basic/pages/dynamic/nested.js
@@ -0,0 +1,5 @@
+import dynamic from 'next/dynamic'
+
+const DynamicComponent = dynamic(() => import('../../components/nested1'))
+
+export default DynamicComponent
diff --git a/test/integration/basic/test/dynamic.js b/test/integration/basic/test/dynamic.js
index 0331616d..c81e578b 100644
--- a/test/integration/basic/test/dynamic.js
+++ b/test/integration/basic/test/dynamic.js
@@ -37,6 +37,26 @@ export default (context, render) => {
         }
       })
 
+      it('should hydrate nested chunks', async () => {
+        let browser
+        try {
+          browser = await webdriver(context.appPort, '/dynamic/nested')
+          await check(() => browser.elementByCss('body').text(), /Nested 1/)
+          await check(() => browser.elementByCss('body').text(), /Nested 2/)
+          await check(() => browser.elementByCss('body').text(), /Browser hydrated/)
+
+          const logs = await browser.log('browser')
+
+          logs.forEach(logItem => {
+            expect(logItem.message).not.toMatch(/Expected server HTML to contain/)
+          })
+        } finally {
+          if (browser) {
+            browser.close()
+          }
+        }
+      })
+
       it('should render the component Head content', async () => {
         let browser
         try {