Skip to content

Commit

Permalink
fix(server): disable common chunks optim
Browse files Browse the repository at this point in the history
  • Loading branch information
gregberge committed Oct 31, 2018
1 parent cfcdb17 commit 78e7b28
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 19 deletions.
59 changes: 49 additions & 10 deletions packages/babel-plugin/src/__snapshots__/index.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -138,7 +138,7 @@ exports[`plugin loadable.lib should be transpiled too 1`] = `
},
requireAsync: () => import(
/* webpackChunkName: \\"moment\\" */
/* webpackChunkName: \\"loadable-moment\\" */
'moment'),
requireSync(props) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions packages/babel-plugin/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
14 changes: 10 additions & 4 deletions packages/babel-plugin/src/properties/chunkName.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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'),
Expand Down
23 changes: 18 additions & 5 deletions packages/server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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()],
}
```

Expand Down Expand Up @@ -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'
Expand All @@ -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'
Expand Down
4 changes: 4 additions & 0 deletions packages/webpack-plugin/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 78e7b28

Please sign in to comment.