From 42736c061ad0e5610522de2517c928b2b8af0ed4 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Tue, 25 Sep 2018 15:27:09 +0200 Subject: [PATCH] Introduce dynamic(() => import()) (#5249) * Add failing tests * Upgrade wd module * Pass dynamic import webpack ids to the client side * Pass through webpack ids to initalializer and only use those * Compile dynamic(import()) to dynamic(() => import()) * Default dynamicIds * Use forked hard-source-plugin * Possibly fix test * Make tests fail less intermittently * Temporarily disable hard-source in production * Make sure dynamic import chunks are unique * Disable hard-source * Log html if error is thrown * Fix test --- build/babel/plugins/react-loadable-plugin.js | 8 ++++ build/webpack.js | 5 ++- client/index.js | 5 ++- lib/dynamic.js | 6 +++ lib/loadable.js | 38 ++++++++++++++---- package.json | 6 +-- server/render.js | 4 +- test/integration/basic/components/hello3.js | 3 ++ test/integration/basic/components/hello4.js | 3 ++ .../basic/pages/dynamic/function.js | 5 +++ .../basic/pages/dynamic/multiple-modules.js | 17 ++++++++ test/integration/basic/pages/dynamic/ssr.js | 1 - test/integration/basic/test/dynamic.js | 40 +++++++++++++++++++ test/integration/basic/test/error-recovery.js | 18 ++++----- .../integration/production/test/index.test.js | 3 +- 15 files changed, 134 insertions(+), 28 deletions(-) create mode 100644 test/integration/basic/components/hello3.js create mode 100644 test/integration/basic/components/hello4.js create mode 100644 test/integration/basic/pages/dynamic/function.js create mode 100644 test/integration/basic/pages/dynamic/multiple-modules.js diff --git a/build/babel/plugins/react-loadable-plugin.js b/build/babel/plugins/react-loadable-plugin.js index 29159ac4..100e2e33 100644 --- a/build/babel/plugins/react-loadable-plugin.js +++ b/build/babel/plugins/react-loadable-plugin.js @@ -139,6 +139,14 @@ export default function ({ types: t, template }) { ]) ) ) + + // Turns `dynamic(import('something'))` into `dynamic(() => import('something'))` for backwards compat. + // This is the replicate the behavior in versions below Next.js 7 where we magically handled not executing the `import()` too. + // We'll deprecate this behavior and provide a codemod for it in 7.1. + if (loader.isCallExpression()) { + const arrowFunction = t.arrowFunctionExpression([], loader.node) + loader.replaceWith(arrowFunction) + } }) } } diff --git a/build/webpack.js b/build/webpack.js index c9930a47..b82abb1d 100644 --- a/build/webpack.js +++ b/build/webpack.js @@ -20,7 +20,7 @@ import { ReactLoadablePlugin } from './webpack/plugins/react-loadable-plugin' import {SERVER_DIRECTORY, NEXT_PROJECT_ROOT, NEXT_PROJECT_ROOT_NODE_MODULES, NEXT_PROJECT_ROOT_DIST, DEFAULT_PAGES_DIR, REACT_LOADABLE_MANIFEST, CLIENT_STATIC_FILES_RUNTIME_WEBPACK, CLIENT_STATIC_FILES_RUNTIME_MAIN} from '../lib/constants' import AutoDllPlugin from 'autodll-webpack-plugin' import TerserPlugin from 'terser-webpack-plugin' -import HardSourceWebpackPlugin from 'hard-source-webpack-plugin' +// import HardSourceWebpackPlugin from '@zeit/hard-source-webpack-plugin' // The externals config makes sure that // on the server side when modules are @@ -241,7 +241,8 @@ export default async function getBaseWebpackConfig (dir: string, {dev = false, i resolve: resolveConfig } }), - new HardSourceWebpackPlugin(), + // Temporarily only enabled in development + // dev && new HardSourceWebpackPlugin(), // This plugin makes sure `output.filename` is used for entry chunks new ChunkNamesPlugin(), !isServer && new ReactLoadablePlugin({ diff --git a/client/index.js b/client/index.js index 858ca911..c8d0eef6 100644 --- a/client/index.js +++ b/client/index.js @@ -28,7 +28,8 @@ const { query, buildId, assetPrefix, - runtimeConfig + runtimeConfig, + dynamicIds }, location } = window @@ -90,7 +91,7 @@ export default async ({ initialErr = error } - await Loadable.preloadReady() + await Loadable.preloadReady(dynamicIds || []) router = createRouter(pathname, query, asPath, { initialProps: props, diff --git a/lib/dynamic.js b/lib/dynamic.js index 0dc6df62..d41edd17 100644 --- a/lib/dynamic.js +++ b/lib/dynamic.js @@ -71,8 +71,14 @@ export default function dynamic (dynamicOptions: any, options: NextDynamicOption } // Support for direct import(), eg: dynamic(import('../hello-world')) + // Note that this is only kept for the edge case where someone is passing in a promise as first argument + // The react-loadable babel plugin will turn dynamic(import('../hello-world')) into dynamic(() => import('../hello-world')) + // To make sure we don't execute the import without rendering first if (typeof dynamicOptions.then === 'function') { loadableOptions.loader = () => dynamicOptions + // Support for having import as a function, eg: dynamic(() => import('../hello-world')) + } else if (typeof dynamicOptions === 'function') { + loadableOptions.loader = dynamicOptions // Support for having first argument being options, eg: dynamic({loader: import('../hello-world')}) } else if (typeof dynamicOptions === 'object') { loadableOptions = {...loadableOptions, ...dynamicOptions} diff --git a/lib/loadable.js b/lib/loadable.js index 159103dd..dcf87fe2 100644 --- a/lib/loadable.js +++ b/lib/loadable.js @@ -25,7 +25,8 @@ import React from 'react' import PropTypes from 'prop-types' const ALL_INITIALIZERS = [] -const READY_INITIALIZERS = [] +const READY_INITIALIZERS = new Map() +let initialized = false function load (loader) { let promise = loader() @@ -129,12 +130,19 @@ function createLoadableComponent (loadFn, options) { return res.promise } - ALL_INITIALIZERS.push(init) + // Server only + if (typeof window === 'undefined') { + ALL_INITIALIZERS.push(init) + } - if (typeof opts.webpack === 'function') { - READY_INITIALIZERS.push(() => { - return init() - }) + // 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() + }) + } } return class LoadableComponent extends React.Component { @@ -286,10 +294,24 @@ Loadable.preloadAll = () => { }) } -Loadable.preloadReady = () => { +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() + // We always will resolve, errors should be handled within loading UIs. - flushInitializers(READY_INITIALIZERS).then(resolve, resolve) + flushInitializers(initializers).then(resolve, resolve) }) } diff --git a/package.json b/package.json index 62314637..5b069ac4 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "@babel/runtime": "7.0.0", "@babel/runtime-corejs2": "7.0.0", "@babel/template": "7.0.0", + "@zeit/hard-source-webpack-plugin": "0.13.0", "ansi-html": "0.0.7", "autodll-webpack-plugin": "0.4.2", "babel-core": "7.0.0-bridge.0", @@ -86,7 +87,6 @@ "fresh": "0.5.2", "friendly-errors-webpack-plugin": "1.7.0", "glob": "7.1.2", - "hard-source-webpack-plugin": "0.12.0", "hoist-non-react-statics": "2.5.5", "htmlescape": "1.1.1", "http-errors": "1.6.2", @@ -109,7 +109,7 @@ "terser-webpack-plugin": "1.0.2", "unfetch": "3.0.0", "url": "0.11.0", - "webpack": "4.19.0", + "webpack": "4.18.1", "webpack-dev-middleware": "3.2.0", "webpack-hot-middleware": "2.22.3", "webpack-sources": "1.2.0", @@ -151,7 +151,7 @@ "rimraf": "2.6.2", "standard": "11.0.1", "taskr": "1.1.0", - "wd": "1.4.1" + "wd": "1.10.3" }, "peerDependencies": { "react": "^16.0.0", diff --git a/server/render.js b/server/render.js index f82a7f65..115970fb 100644 --- a/server/render.js +++ b/server/render.js @@ -157,7 +157,8 @@ async function doRender (req, res, pathname, query, { await Loadable.preloadAll() // Make sure all dynamic imports are loaded const docProps = await loadGetInitialProps(Document, { ...ctx, renderPage }) - const dynamicImports = getDynamicImportBundles(reactLoadableManifest, reactLoadableModules) + const dynamicImports = [...(new Set(getDynamicImportBundles(reactLoadableManifest, reactLoadableModules)))] + const dynamicImportsIds = dynamicImports.map((bundle) => bundle.id) if (isResSent(res)) return @@ -172,6 +173,7 @@ async function doRender (req, res, pathname, query, { assetPrefix: assetPrefix === '' ? undefined : assetPrefix, // send assetPrefix to the client side when configured, otherwise don't sent in the resulting HTML runtimeConfig, // runtimeConfig if provided, otherwise don't sent in the resulting HTML nextExport, // If this is a page exported by `next export` + dynamicIds: dynamicImportsIds.length === 0 ? undefined : dynamicImportsIds, err: (err) ? serializeError(dev, err) : undefined // Error if one happened, otherwise don't sent in the resulting HTML }, dev, diff --git a/test/integration/basic/components/hello3.js b/test/integration/basic/components/hello3.js new file mode 100644 index 00000000..a896f64f --- /dev/null +++ b/test/integration/basic/components/hello3.js @@ -0,0 +1,3 @@ +export default () => ( +

Hello World 1

+) diff --git a/test/integration/basic/components/hello4.js b/test/integration/basic/components/hello4.js new file mode 100644 index 00000000..b92721c2 --- /dev/null +++ b/test/integration/basic/components/hello4.js @@ -0,0 +1,3 @@ +export default () => ( +

Hello World 2

+) diff --git a/test/integration/basic/pages/dynamic/function.js b/test/integration/basic/pages/dynamic/function.js new file mode 100644 index 00000000..c015ff1a --- /dev/null +++ b/test/integration/basic/pages/dynamic/function.js @@ -0,0 +1,5 @@ +import dynamic from 'next/dynamic' + +const Hello = dynamic(() => import('../../components/hello1')) + +export default Hello diff --git a/test/integration/basic/pages/dynamic/multiple-modules.js b/test/integration/basic/pages/dynamic/multiple-modules.js new file mode 100644 index 00000000..06d3eae0 --- /dev/null +++ b/test/integration/basic/pages/dynamic/multiple-modules.js @@ -0,0 +1,17 @@ +/* eslint-disable */ +import dynamic from 'next/dynamic' + +const Hello = dynamic(import(/* webpackChunkName: 'hello1' */ '../../components/hello3')) +const Hello2 = dynamic(import(/* webpackChunkName: 'hello2' */ '../../components/hello4')) + +export default () => { + return
+ + + + + + + +
+} \ No newline at end of file diff --git a/test/integration/basic/pages/dynamic/ssr.js b/test/integration/basic/pages/dynamic/ssr.js index 062b8ecc..6bec25ce 100644 --- a/test/integration/basic/pages/dynamic/ssr.js +++ b/test/integration/basic/pages/dynamic/ssr.js @@ -1,4 +1,3 @@ - import dynamic from 'next/dynamic' const Hello = dynamic(import('../../components/hello1')) diff --git a/test/integration/basic/test/dynamic.js b/test/integration/basic/test/dynamic.js index 1114fdf1..48f94e85 100644 --- a/test/integration/basic/test/dynamic.js +++ b/test/integration/basic/test/dynamic.js @@ -15,6 +15,11 @@ export default (context, render) => { expect($('body').text()).toMatch(/Hello World 1/) }) + it('should render dynamic import components using a function as first parameter', async () => { + const $ = await get$('/dynamic/function') + expect($('body').text()).toMatch(/Hello World 1/) + }) + it('should render even there are no physical chunk exists', async () => { let browser try { @@ -102,6 +107,41 @@ export default (context, render) => { }) }) + describe('Multiple modules', () => { + it('should only include the rendered module script tag', async () => { + const $ = await get$('/dynamic/multiple-modules') + const html = $('html').html() + expect(html).toMatch(/hello1\.js/) + expect(html).not.toMatch(/hello2\.js/) + }) + + it('should only load the rendered module in the browser', async () => { + let browser + try { + browser = await webdriver(context.appPort, '/dynamic/multiple-modules') + const html = await browser.elementByCss('html').getAttribute('innerHTML') + expect(html).toMatch(/hello1\.js/) + expect(html).not.toMatch(/hello2\.js/) + } finally { + if (browser) { + browser.close() + } + } + }) + + it('should only render one bundle if component is used multiple times', async () => { + const $ = await get$('/dynamic/multiple-modules') + const html = $('html').html() + try { + expect(html.match(/chunks[\\/]hello1\.js/g).length).toBe(2) // one for preload, one for the script tag + expect(html).not.toMatch(/hello2\.js/) + } catch (err) { + console.error(html) + throw err + } + }) + }) + describe('Import mapping', () => { it('should render dynamic imports bundle', async () => { const $ = await get$('/dynamic/bundle') diff --git a/test/integration/basic/test/error-recovery.js b/test/integration/basic/test/error-recovery.js index 12a9da06..1d585375 100644 --- a/test/integration/basic/test/error-recovery.js +++ b/test/integration/basic/test/error-recovery.js @@ -45,7 +45,7 @@ export default (context, render) => { aboutPage.replace('', 'div') - await waitFor(3000) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/Unterminated JSX contents/) @@ -75,11 +75,11 @@ export default (context, render) => { aboutPage.replace('', 'div') - await waitFor(3000) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/Unterminated JSX contents/) - await waitFor(2000) + await waitFor(10000) // Check for the error overlay const bodyHtml = await browser.elementByCss('body').getAttribute('innerHTML') @@ -100,7 +100,7 @@ export default (context, render) => { browser = await webdriver(context.appPort, '/hmr/contact') - await waitFor(3000) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/Unterminated JSX contents/) @@ -129,7 +129,7 @@ export default (context, render) => { aboutPage.replace('export', 'aa=20;\nexport') - await waitFor(3000) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/aa is not defined/) @@ -156,7 +156,7 @@ export default (context, render) => { const aboutPage = new File(join(__dirname, '../', 'pages', 'hmr', 'about.js')) aboutPage.replace('return', 'throw new Error("an-expected-error");\nreturn') - await waitFor(3000) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/an-expected-error/) @@ -179,7 +179,7 @@ export default (context, render) => { const aboutPage = new File(join(__dirname, '../', 'pages', 'hmr', 'about.js')) aboutPage.replace('export default', 'export default "not-a-page"\nexport const fn = ') - await waitFor(3000) + await waitFor(10000) expect(await browser.elementByCss('body').text()).toMatch(/The default export is not a React Component/) @@ -221,7 +221,7 @@ export default (context, render) => { const browser = await webdriver(context.appPort, '/hmr') await browser.elementByCss('#error-in-gip-link').click() - await waitFor(1500) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/an-expected-error-in-gip/) @@ -240,7 +240,7 @@ export default (context, render) => { it('should recover after an error reported via SSR', async () => { const browser = await webdriver(context.appPort, '/hmr/error-in-gip') - await waitFor(1500) + await waitFor(10000) expect(await getReactErrorOverlayContent(browser)).toMatch(/an-expected-error-in-gip/) diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index b16b2df8..1b78a0d0 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -73,8 +73,7 @@ describe('Production Usage', () => { resources.push(`${url}static/${buildId}/pages/index.js`) // test dynamic chunk - const file = Object.keys(reactLoadableManifest).find((i) => i.indexOf('components/hello1') !== -1) - resources.push(url + reactLoadableManifest[file][0].publicPath) + resources.push(url + reactLoadableManifest['../../components/hello1'][0].publicPath) // test main.js runtime etc for (const item of buildManifest.pages['/']) {