From 78e7b2816cbd7e2222c8ade2621abe300fbe86de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Wed, 31 Oct 2018 15:45:36 +0100 Subject: [PATCH] fix(server): disable common chunks optim --- .../src/__snapshots__/index.test.js.snap | 59 +++++++++++++++---- packages/babel-plugin/src/index.test.js | 8 +++ .../babel-plugin/src/properties/chunkName.js | 14 +++-- packages/server/README.md | 23 ++++++-- packages/webpack-plugin/src/index.js | 4 ++ 5 files changed, 89 insertions(+), 19 deletions(-) diff --git a/packages/babel-plugin/src/__snapshots__/index.test.js.snap b/packages/babel-plugin/src/__snapshots__/index.test.js.snap index 789df1b1f..cfa1996c5 100644 --- a/packages/babel-plugin/src/__snapshots__/index.test.js.snap +++ b/packages/babel-plugin/src/__snapshots__/index.test.js.snap @@ -126,7 +126,7 @@ exports[`plugin aggressive import without "webpackChunkName" should add it 1`] = exports[`plugin loadable.lib should be transpiled too 1`] = ` "loadable.lib({ chunkName() { - return \\"moment\\"; + return \\"loadable-moment\\"; }, isReady(props) { @@ -138,7 +138,7 @@ exports[`plugin loadable.lib should be transpiled too 1`] = ` }, requireAsync: () => import( - /* webpackChunkName: \\"moment\\" */ + /* webpackChunkName: \\"loadable-moment\\" */ 'moment'), requireSync(props) { @@ -165,7 +165,7 @@ exports[`plugin loadable.lib should be transpiled too 1`] = ` exports[`plugin simple import in a complex promise should work 1`] = ` "loadable({ chunkName() { - return \\"ModA\\"; + return \\"loadable-ModA\\"; }, isReady(props) { @@ -177,7 +177,7 @@ exports[`plugin simple import in a complex promise should work 1`] = ` }, requireAsync: () => timeout(import( - /* webpackChunkName: \\"ModA\\" */ + /* webpackChunkName: \\"loadable-ModA\\" */ './ModA'), 2000), requireSync(props) { @@ -204,7 +204,7 @@ exports[`plugin simple import in a complex promise should work 1`] = ` exports[`plugin simple import should transform path into "chunk-friendly" name 1`] = ` "loadable({ chunkName() { - return \\"foo-bar\\"; + return \\"loadable-foo-bar\\"; }, isReady(props) { @@ -216,7 +216,7 @@ exports[`plugin simple import should transform path into "chunk-friendly" name 1 }, requireAsync: () => import( - /* webpackChunkName: \\"foo-bar\\" */ + /* webpackChunkName: \\"loadable-foo-bar\\" */ '../foo/bar'), requireSync(props) { @@ -279,10 +279,49 @@ exports[`plugin simple import should work with template literal 1`] = ` });" `; +exports[`plugin simple import with "webpackChunkName" comment should not put prefix if already there 1`] = ` +"loadable({ + chunkName() { + return \\"loadable-ChunkA\\"; + }, + + isReady(props) { + if (typeof __webpack_modules__ !== 'undefined') { + return !!__webpack_modules__[this.resolve(props)]; + } + + return false; + }, + + requireAsync: () => import( + /* webpackChunkName: \\"loadable-ChunkA\\" */ + './ModA'), + + requireSync(props) { + const id = this.resolve(props); + + if (typeof __webpack_require__ !== 'undefined') { + return __webpack_require__(id); + } + + return eval('module.require')(id); + }, + + resolve() { + if (require.resolveWeak) { + return require.resolveWeak(\\"./ModA\\"); + } + + return require('path').resolve(__dirname, \\"./ModA\\"); + } + +});" +`; + exports[`plugin simple import with "webpackChunkName" comment should use it 1`] = ` "loadable({ chunkName() { - return \\"ChunkA\\"; + return \\"loadable-ChunkA\\"; }, isReady(props) { @@ -294,7 +333,7 @@ exports[`plugin simple import with "webpackChunkName" comment should use it 1`] }, requireAsync: () => import( - /* webpackChunkName: \\"ChunkA\\" */ + /* webpackChunkName: \\"loadable-ChunkA\\" */ './ModA'), requireSync(props) { @@ -321,7 +360,7 @@ exports[`plugin simple import with "webpackChunkName" comment should use it 1`] exports[`plugin simple import without "webpackChunkName" comment should add it 1`] = ` "loadable({ chunkName() { - return \\"ModA\\"; + return \\"loadable-ModA\\"; }, isReady(props) { @@ -333,7 +372,7 @@ exports[`plugin simple import without "webpackChunkName" comment should add it 1 }, requireAsync: () => import( - /* webpackChunkName: \\"ModA\\" */ + /* webpackChunkName: \\"loadable-ModA\\" */ './ModA'), requireSync(props) { diff --git a/packages/babel-plugin/src/index.test.js b/packages/babel-plugin/src/index.test.js index fb96d5581..728ea808c 100644 --- a/packages/babel-plugin/src/index.test.js +++ b/packages/babel-plugin/src/index.test.js @@ -37,6 +37,14 @@ describe('plugin', () => { expect(result).toMatchSnapshot() }) + + it('should not put prefix if already there', () => { + const result = testPlugin(` + loadable(() => import(/* webpackChunkName: "loadable-ChunkA" */ './ModA')) + `) + + expect(result).toMatchSnapshot() + }) }) describe('without "webpackChunkName" comment', () => { diff --git a/packages/babel-plugin/src/properties/chunkName.js b/packages/babel-plugin/src/properties/chunkName.js index 490fd2006..8beaa136f 100644 --- a/packages/babel-plugin/src/properties/chunkName.js +++ b/packages/babel-plugin/src/properties/chunkName.js @@ -65,14 +65,17 @@ export default function chunkNameProperty({ types: t }) { importArg.node.expressions, ) } - return t.stringLiteral(moduleToChunk(importArg.node.value)) + return t.stringLiteral(`loadable-${moduleToChunk(importArg.node.value)}`) } function getExistingChunkName(callPath) { const importArg = getImportArg(callPath) const chunkName = getRawChunkNameFromCommments(importArg) if (!chunkName) return null - return t.stringLiteral(chunkName) + const loadableChunkName = chunkName.startsWith('loadable-') + ? chunkName + : `loadable-${chunkName}` + return t.stringLiteral(loadableChunkName) } function isAgressiveImport(callPath) { @@ -82,7 +85,7 @@ export default function chunkNameProperty({ types: t }) { ) } - function addChunkNameComment(callPath, chunkName) { + function addOrReplaceChunkNameComment(callPath, chunkName) { const importArg = getImportArg(callPath) const chunkNameComment = getChunkNameComment(importArg) if (chunkNameComment) { @@ -104,14 +107,17 @@ export default function chunkNameProperty({ types: t }) { return ({ callPath, funcPath }) => { let chunkName const agressiveImport = isAgressiveImport(callPath) + if (!agressiveImport) { chunkName = getExistingChunkName(callPath) } + if (!chunkName) { chunkName = generateChunkName(callPath) - addChunkNameComment(callPath, chunkName) } + addOrReplaceChunkNameComment(callPath, chunkName) + return t.objectMethod( 'method', t.identifier('chunkName'), diff --git a/packages/server/README.md b/packages/server/README.md index f2a8fd80d..e42356146 100644 --- a/packages/server/README.md +++ b/packages/server/README.md @@ -17,7 +17,7 @@ This table compares the two modes. | Mode | `@loadable/babel-plugin` | `@loadable/webpack-plugin` | `loadComponents()` | Perf | | ------- | ------------------------ | -------------------------- | ------------------ | ---- | | Static | Required | Required | Not required | ++ | -| Dynamic | Required | No required | Required | + | +| Dynamic | Required | Required | Required | + | ### Static Mode (recommended) @@ -39,8 +39,8 @@ This table compares the two modes. const LoadablePlugin = require('@loadable/webpack-plugin') module.exports = { - /* Your webpack config... */ - plugins: [new HtmlWebpackPlugin()], + // ... + plugins: [new LoadablePlugin()], } ``` @@ -75,7 +75,20 @@ const scriptTags = loadableState.getScriptTags() // or loadableState.getScriptEl } ``` -#### 2. Setup `LoadableState` server-side (without manifest) +#### 2. Install `@loadable/webpack-plugin` + +**webpack.config.js** + +```js +const LoadablePlugin = require('@loadable/webpack-plugin') + +module.exports = { + // ... + plugins: [new LoadablePlugin()], +} +``` + +#### 3. Setup `LoadableState` server-side (without manifest) ```js import path from 'path' @@ -92,7 +105,7 @@ const html = renderToString( const scriptTags = loadableState.getScriptTags() // or loadableState.getScriptElements(); ``` -#### 3. Use `loadComponents()` client-side +#### 4. Use `loadComponents()` client-side ```js import { hydrate } from 'react-dom' diff --git a/packages/webpack-plugin/src/index.js b/packages/webpack-plugin/src/index.js index 955164700..255f87c4d 100644 --- a/packages/webpack-plugin/src/index.js +++ b/packages/webpack-plugin/src/index.js @@ -6,6 +6,10 @@ class LoadablePlugin { } apply(compiler) { + // Disable splitChunks for loadable chunks + compiler.options.optimization.splitChunks.chunks = chunk => + !chunk.canBeInitial() && !chunk.name.startsWith('loadable-') + compiler.hooks.emit.tap('@loadable/webpack-plugin', hookCompiler => { const { assetsByChunkName, publicPath } = hookCompiler.getStats().toJson({ hash: true,