Skip to content

Commit

Permalink
fix: reference webpack chunks via chunk name
Browse files Browse the repository at this point in the history
We observed that in some cases the generated module ids differe between
the browser and server build.
This happens when one of the builds is able to concatenate modules into
one chunk while the other build can't (e.g. because the modules live in
different chunks because of code splitting).
Because we did not want to split the server build into chunks and then
load them manually, we decided to generate stable chunk names, that do
not differ between browser/server, in our importComponent babel plugin.

We tried to work around this issue previously via #1976 but with a later
webpack version our workaround stopped working.
Furthermore, our previous workaround had other issues, which impacted
tree-shaking and therefore the bundle size.

Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Philipp Hinrichsen <philipp.hinrichsen@new-work.se>
Co-authored-by: Robert Kowalski <robert.kowalski@new-work.se>
  • Loading branch information
4 people committed Feb 16, 2022
1 parent de003f2 commit 3521a78
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 68 deletions.
73 changes: 68 additions & 5 deletions packages/react/import-component/babel.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,49 @@
'use strict';

const nodePath = require('path');
const crypto = require('crypto');

/**
* This plugin transforms the following function call:
* ```javascript
* importComponent(() => import('./component'));
* ```
*
* Into the following:
*
* ```javascript
* importComponent({
* load: () => import('./component'),
* moduleId: require.resolveWeak('./component'),
* chunkName: () => 'hash'
* })
* ```
*/

const regex = /webpackChunkName:\s*(?:(['"])([^'"]*)\1|([^\s]*))/;

function extractChunkName(t, leadingComments) {
if (!leadingComments || !Array.isArray(leadingComments)) {
return;
}

const comment = leadingComments.find((comment) =>
comment.value.includes('webpackChunkName')
);

if (!comment) {
return;
}

const results = comment.value.match(regex);
const cleanChunkName = results[2] || results[3];

return cleanChunkName;
}

module.exports = ({ types: t }) => ({
visitor: {
ImportDeclaration(path) {
ImportDeclaration(path, state) {
const modules = ['hops-react', this.opts.module];
const source = path.node.source.value;
if (!modules.includes(source)) return;
Expand Down Expand Up @@ -31,19 +72,37 @@ module.exports = ({ types: t }) => ({
t.assertCallExpression(argument.get('body'));
t.assertImport(argument.get('body.callee'));

const { node } = argument.get('body.arguments.0');
const importedComponent = node.value;
const leadingComments = node.leadingComments;
const argPath = argument.get('body.arguments.0');
const importedComponent = argPath.node.value;
const resourcePathNode = t.stringLiteral(importedComponent);

let chunkName = extractChunkName(t, argPath.node.leadingComments);
if (!chunkName) {
const hasher = crypto.createHash('md4');
hasher.update(nodePath.relative(this.opts.rootDir, state.filename));
hasher.update(importedComponent);
const hash = hasher.digest('base64').slice(0, 4);

t.addComment(
argPath.node,
'leading',
` webpackChunkName: '${hash}' `
);
chunkName = hash;
}

argument.replaceWith(
t.objectExpression([
t.objectProperty(
t.identifier('load'),
t.arrowFunctionExpression(
[],
t.callExpression(t.identifier('import'), [
t.addComments(resourcePathNode, 'leading', leadingComments),
t.addComments(
resourcePathNode,
'leading',
argPath.node.leadingComments
),
])
)
),
Expand All @@ -57,6 +116,10 @@ module.exports = ({ types: t }) => ({
[resourcePathNode]
)
),
t.objectProperty(
t.identifier('chunkName'),
t.arrowFunctionExpression([], t.stringLiteral(chunkName))
),
])
);
});
Expand Down
14 changes: 6 additions & 8 deletions packages/react/import-component/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import PropTypes from 'prop-types';
import ImportComponentContext from './context';

export const importComponent = (
{ load, moduleId },
{ load, moduleId, chunkName },
resolve = (module) => module.default
) => {
class Importer extends Component {
constructor({ hasModules }) {
constructor() {
super();
if (hasModules || __webpack_modules__[moduleId]) {
if (__webpack_modules__[moduleId]) {
this.state = { Component: resolve(__webpack_require__(moduleId)) };
} else {
this.state = { loading: true };
Expand Down Expand Up @@ -57,21 +57,19 @@ export const importComponent = (
}

Importer.propTypes = {
hasModules: PropTypes.bool,
ownProps: PropTypes.object.isRequired,
loader: PropTypes.func,
render: PropTypes.func,
};

function Import({ loader, render, ...ownProps }) {
const modules = useContext(ImportComponentContext);
const chunks = useContext(ImportComponentContext);

if (modules) {
modules.push(moduleId);
if (chunks) {
chunks.push(chunkName());
}

return createElement(Importer, {
hasModules: Boolean(modules && modules.length > 0),
loader,
render,
ownProps,
Expand Down
2 changes: 1 addition & 1 deletion packages/react/import-component/mixin.core.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ImportComponentCoreMixin extends Mixin {
} else {
jsLoaderConfig.options.plugins.push([
require.resolve('../lib/babel'),
{ module: 'hops' },
{ module: 'hops', rootDir: this.config.rootDir },
]);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react/import-component/mixin.runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import { Provider } from './context';
class ImportComponentMixin extends Mixin {
bootstrap(_req, res) {
if (res) {
res.locals.modules = this.modules = [];
res.locals.chunks = this.chunks = [];
}
}

enhanceElement(element) {
return createElement(Provider, { value: this.modules }, element);
return createElement(Provider, { value: this.chunks }, element);
}
}

Expand Down
18 changes: 11 additions & 7 deletions packages/react/lib/assets.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
'use strict';

import { extname } from 'path';
import { ensureLeadingSlash } from 'pathifist';

export default (stats, modules) => {
const { entryFiles, vendorFiles, moduleFileMap } = stats;
const moduleFiles = modules.reduce(
(result, module) => [...result, ...moduleFileMap[module]],
[]
);
export default (stats, chunks) => {
const { entryFiles, vendorFiles } = stats;
const chunkFiles = chunks.reduce((result, chunk) => {
const chunkGroup = stats.namedChunkGroups[chunk];
const assets = chunkGroup.assets.map((asset) =>
ensureLeadingSlash(asset.name)
);
return [...result, ...assets];
}, []);

return [...vendorFiles, ...moduleFiles, ...entryFiles]
return [...vendorFiles, ...chunkFiles, ...entryFiles]
.filter(
(asset, index, self) =>
self.indexOf(asset) === index &&
Expand Down
8 changes: 4 additions & 4 deletions packages/react/render/mixin.server.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class ReactMixin extends Mixin {
return renderToFragments(element);
}

renderTemplate(fragments, { modules }) {
const assets = getAssets(this.stats, modules);
renderTemplate(fragments, { chunks }) {
const assets = getAssets(this.stats, chunks);
const resourceHints = getResourceHints(this.stats);
const globals = { _env: this.config._env };

Expand Down Expand Up @@ -70,9 +70,9 @@ class ReactMixin extends Mixin {
} else {
res.status(router.status || 200);

// note: res.locals.modules is set by the ImportComponentMixin
// note: res.locals.chunks is set by the ImportComponentMixin
return this.renderTemplate(fragments, {
modules: res.locals.modules,
chunks: res.locals.chunks,
}).then((page) => res.send(page));
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/spec/integration/react/__tests__/develop.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ describe('react development server', () => {
const preloadAs = await getProperty('as', 'link[rel="preload"]');
const prefetchHref = await getProperty('href', 'link[rel="prefetch"]');

expect(preloadHref).toMatch(/\/fixture-react-[0-9].js$/);
expect(preloadHref).toMatch(/\/fixture-react-[^.]+.js$/);
expect(preloadAs).toBe('script');
expect(prefetchHref).toMatch(/\/fixture-react-[0-9].js$/);
expect(prefetchHref).toMatch(/\/fixture-react-[^.]+.js$/);

await page.close();
});
Expand Down
11 changes: 1 addition & 10 deletions packages/webpack/lib/configs/build.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
const { dirname, relative } = require('path');
const { EnvironmentPlugin, DefinePlugin, ids } = require('webpack');
const { EnvironmentPlugin, DefinePlugin } = require('webpack');
const TerserPlugin = require('terser-webpack-plugin');
const { join, trimSlashes } = require('pathifist');
const getModules = require('../utils/modules');

const { HashedModuleIdsPlugin, NamedModuleIdsPlugin } = ids;

module.exports = function getConfig(config, name, buildDependencies) {
const getAssetPath = (...arg) => trimSlashes(join(config.assetPath, ...arg));
const isProduction = process.env.NODE_ENV === 'production';
Expand Down Expand Up @@ -131,9 +129,6 @@ module.exports = function getConfig(config, name, buildDependencies) {
},
externals: [],
optimization: {
splitChunks: {
chunks: 'all',
},
minimizer: [
new TerserPlugin({
extractComments: false,
Expand All @@ -142,12 +137,8 @@ module.exports = function getConfig(config, name, buildDependencies) {
},
}),
],
chunkIds: 'natural',
usedExports: false,
concatenateModules: true,
},
plugins: [
new (isProduction ? HashedModuleIdsPlugin : NamedModuleIdsPlugin)(),
// Needed for bootstrap/lib/utils#environmentalize, which falls
// back to `process.env` if there's no global variable `_env`
new DefinePlugin({ 'process.env': JSON.stringify({}) }),
Expand Down
6 changes: 1 addition & 5 deletions packages/webpack/lib/configs/develop.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,7 @@ module.exports = function getConfig(config, name, buildDependencies) {
rules: [{ oneOf: allLoaderConfigs }],
},
externals: [],
optimization: {
splitChunks: { chunks: 'all' },
moduleIds: 'named',
chunkIds: 'natural',
},
optimization: {},
plugins: [
// Needed for bootstrap/lib/utils#environmentalize, which falls
// back to `process.env` if there's no global variable `_env`
Expand Down
5 changes: 0 additions & 5 deletions packages/webpack/lib/configs/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ const {
EnvironmentPlugin,
HotModuleReplacementPlugin,
optimize,
ids,
} = require('webpack');
const { join, trimSlashes } = require('pathifist');
const getModules = require('../utils/modules');

const { LimitChunkCountPlugin } = optimize;
const { HashedModuleIdsPlugin, NamedModuleIdsPlugin } = ids;

module.exports = function getConfig(config, name, buildDependencies) {
const getAssetPath = (...arg) => trimSlashes(join(config.assetPath, ...arg));
Expand Down Expand Up @@ -155,12 +153,9 @@ module.exports = function getConfig(config, name, buildDependencies) {
externals: [],
optimization: {
minimizer: [],
chunkIds: 'natural',
usedExports: false,
},
plugins: [
new LimitChunkCountPlugin({ maxChunks: 1 }),
new (isProduction ? HashedModuleIdsPlugin : NamedModuleIdsPlugin)(),
isProduction ? { apply: () => {} } : new HotModuleReplacementPlugin(),
new EnvironmentPlugin({ NODE_ENV: 'development' }),
],
Expand Down
22 changes: 3 additions & 19 deletions packages/webpack/lib/plugins/stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,13 @@ const { trimTrailingSlash } = require('pathifist');

const analyzeCompilation = ({ chunks, chunkGroups, chunkGraph }) => {
const entryChunks = [];
const chunksByModule = [];

for (const chunk of chunks) {
for (const module of chunkGraph.getChunkModulesIterable(chunk)) {
if (chunkGraph.isEntryModule(module)) {
entryChunks.push(chunk);
continue;
}

const moduleId = chunkGraph.getModuleId(module);

if (!moduleId) {
continue;
}

const { chunks } = chunkGroups.find(({ chunks }) =>
chunks.includes(chunk)
);

chunksByModule.push([moduleId, chunks]);
}
}

Expand All @@ -37,12 +24,12 @@ const analyzeCompilation = ({ chunks, chunkGroups, chunkGraph }) => {
[]
);

return { entryChunks, vendorChunks, chunksByModule };
return { entryChunks, vendorChunks };
};

const extractFiles = (chunkData, rawPublicPath) => {
const publicPath = trimTrailingSlash(rawPublicPath);
const { entryChunks, vendorChunks, chunksByModule } = chunkData;
const { entryChunks, vendorChunks } = chunkData;
const gatherFiles = (result, { files }) => [
...result,
...Array.from(files).map((file) => `${publicPath}/${file}`),
Expand All @@ -51,10 +38,6 @@ const extractFiles = (chunkData, rawPublicPath) => {
return {
entryFiles: entryChunks.reduce(gatherFiles, []),
vendorFiles: vendorChunks.reduce(gatherFiles, []),
moduleFileMap: chunksByModule.reduce((result, [module, chunks]) => {
result[module] = chunks.reduce(gatherFiles, []);
return result;
}, {}),
};
};

Expand All @@ -76,6 +59,7 @@ exports.StatsWritePlugin = class StatsWritePlugin {
assets: true,
chunkGroupChildren: true,
entrypoints: true,
chunkGroups: true,
}),
...extractFiles(analyzeCompilation(compilation), publicPath),
};
Expand Down

0 comments on commit 3521a78

Please sign in to comment.