-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(mc-scripts): manually group some vendor chunks #3615
Changes from 11 commits
f7ffaf3
b55c4c6
bad24f8
25a30bc
1c77000
4c27dad
2005f25
ff1069d
c793e3b
8df0afc
f8f062c
ce14413
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
--- | ||
'@commercetools-frontend/mc-scripts': minor | ||
--- | ||
|
||
We're introducing a change the way `build` command works in terms of bundles generation. | ||
We've updated the configuration in order to reduce the size of the application's entry point bundle in favor of three four smaller bundles (which can be downloaded in parallel by the browser). | ||
|
||
This change has been applied to the build process no matter if you use `webpack` or `vite`. | ||
Remember `webpack` is used by default and you can opt-in to `vite` by using this environment variable: | ||
|
||
```bash | ||
ENABLE_EXPERIMENTAL_VITE_BUNDLER=true | ||
``` | ||
|
||
Also, if you are using `vite`, we've also added a couple of plugins you can use to analyze the built bundle sizes: | ||
|
||
- [vite-bundle-analyzer](https://github.com/KusStar/vite-bundle-visualizer) | ||
- [rollup-plugin-visualizer](https://github.com/btd/rollup-plugin-visualizer) (tree visualization) | ||
|
||
You can use them by setting this environment variables: | ||
|
||
```bash | ||
# vite-bundle-analyzer | ||
ANALYZE_BUNDLE=true | ||
# rollup-plugin-visualizer | ||
ANALYZE_BUNDLE_TREE=true | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,7 +181,8 @@ | |
"@typescript-eslint/parser": "^5.52.0" | ||
}, | ||
"patchedDependencies": { | ||
"[email protected]": "patches/[email protected]" | ||
"[email protected]": "patches/[email protected]", | ||
"[email protected]": "patches/[email protected]" | ||
} | ||
}, | ||
"engines": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,6 +100,7 @@ | |
"querystring-es3": "^0.2.1", | ||
"react-dev-utils": "12.0.1", | ||
"react-refresh": "0.14.2", | ||
"rollup-plugin-visualizer": "^5.12.0", | ||
"serve-handler": "6.1.5", | ||
"shelljs": "0.8.5", | ||
"style-loader": "3.3.4", | ||
|
@@ -108,6 +109,7 @@ | |
"thread-loader": "3.0.4", | ||
"url": "^0.11.0", | ||
"vite": "~4.5.3", | ||
"vite-bundle-analyzer": "0.12.1", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if the patch will be used here when someone installs the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair question. Couldn't find documentation on it. I must say I assume it would be applied. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know either but it won't matter in this case: the change in the source code is about a Typescript type which is not generic in the |
||
"webpack": "5.94.0", | ||
"webpack-bundle-analyzer": "4.10.2", | ||
"webpack-dev-server": "4.15.2", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,12 @@ import path from 'path'; | |
import pluginGraphql from '@rollup/plugin-graphql'; | ||
import pluginReact from '@vitejs/plugin-react'; | ||
import fs from 'fs-extra'; | ||
import { build, type Plugin } from 'vite'; | ||
import { visualizer } from 'rollup-plugin-visualizer'; | ||
import { build, PluginOption, type Plugin } from 'vite'; | ||
import { analyzer } from 'vite-bundle-analyzer'; | ||
import { packageLocation as applicationStaticAssetsPath } from '@commercetools-frontend/assets'; | ||
import { generateTemplate } from '@commercetools-frontend/mc-html-template'; | ||
import { getViteCacheGroups } from '../config/optimizations'; | ||
import paths from '../config/paths'; | ||
import pluginDynamicBaseAssetsGlobals from '../vite-plugins/vite-plugin-dynamic-base-assets-globals'; | ||
import pluginI18nMessageCompilation from '../vite-plugins/vite-plugin-i18n-message-compilation'; | ||
|
@@ -28,6 +31,11 @@ async function run() { | |
// Write `index.html` (template) into the `/public` folder. | ||
fs.writeFileSync(paths.appIndexHtml, html, { encoding: 'utf8' }); | ||
|
||
const appDependencies = require(paths.appPackageJson).dependencies as Record< | ||
string, | ||
string | ||
>; | ||
|
||
await build({ | ||
root: paths.appRoot, | ||
base: './', // <-- Important to allow configuring the runtime base path. | ||
|
@@ -43,7 +51,8 @@ async function run() { | |
// NOTE that after the build, Vite will write the `index.html` (template) | ||
// at the `/public/public/index.html` location. See `fs.renameSync` below. | ||
input: paths.appIndexHtml, | ||
// Reduce the memory footpring when building sourcemaps. | ||
output: { manualChunks: getViteCacheGroups(appDependencies) }, | ||
// Reduce the memory footprint when building sourcemaps. | ||
// https://github.com/vitejs/vite/issues/2433#issuecomment-1361094727 | ||
cache: false, | ||
}, | ||
|
@@ -79,6 +88,10 @@ async function run() { | |
pluginSvgr(), | ||
pluginDynamicBaseAssetsGlobals(), | ||
pluginI18nMessageCompilation(), | ||
process.env.ANALYZE_BUNDLE === 'true' && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
analyzer({ defaultSizes: 'parsed', openAnalyzer: true }), | ||
process.env.ANALYZE_BUNDLE_TREE === 'true' && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
(visualizer({ open: true, template: 'network' }) as PluginOption), | ||
], | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import chalk from 'chalk'; | ||
|
||
// Dependencies to be split/grouped into separate chunks. | ||
// This is useful to reduce the main bundle size and have more | ||
// dedicated caching strategy for specific chunks. | ||
const manualChunks = { | ||
'commercetools-uikit-icons': ['@commercetools-uikit/icons'], | ||
moment: ['moment', 'moment-timezone'], | ||
'app-shell': [ | ||
'@commercetools-frontend/application-shell', | ||
'@commercetools-frontend/application-shell-connectors', | ||
], | ||
}; | ||
|
||
const removeNonExistantDependencies = ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just to make sure the packages we're defining in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the comment. I was fearful we'd remove something else but those we specify at first. |
||
manualChunks: Record<string, string[]>, | ||
appDependencies: string[] | ||
) => { | ||
return Object.entries(manualChunks).reduce( | ||
(chunkGroups, [chunkName, vendors]) => { | ||
const existingVendors = vendors.filter((vendor) => | ||
appDependencies.includes(vendor) | ||
); | ||
if (existingVendors.length > 0) { | ||
chunkGroups[chunkName] = existingVendors; | ||
} else { | ||
console.log( | ||
chalk.yellow( | ||
`\nFiltering out chunk "${chunkName}" because its configured dependencies does not exist in the application: ${vendors}\n` | ||
) | ||
); | ||
} | ||
return chunkGroups; | ||
}, | ||
{} as Record<string, string[]> | ||
); | ||
}; | ||
|
||
// Reference: https://rollupjs.org/configuration-options/#output-manualchunks | ||
export function getViteCacheGroups(appDependencies: Record<string, string>) { | ||
return removeNonExistantDependencies( | ||
manualChunks, | ||
Object.keys(appDependencies) | ||
); | ||
} | ||
|
||
// Reference: https://webpack.js.org/plugins/split-chunks-plugin/ | ||
export function getWepbackCacheGroups(appDependencies: Record<string, string>) { | ||
const filteredDependencies = removeNonExistantDependencies( | ||
manualChunks, | ||
Object.keys(appDependencies) | ||
); | ||
|
||
const result = Object.entries(filteredDependencies).reduce( | ||
(previousChunks, [chunkName, vendors]) => { | ||
return { | ||
...previousChunks, | ||
[chunkName]: { | ||
test: new RegExp(`[\\/]node_modules[\\/](${vendors.join('|')})[\\/]`), | ||
name: chunkName, | ||
chunks: 'all', | ||
}, | ||
}; | ||
}, | ||
{} | ||
); | ||
|
||
return result; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
diff --git a/dist/index.d.ts b/dist/index.d.ts | ||
index 6a491b2b03b5bf351e1ff07e2ac8659bd47c54c5..40d52d026125492824f8fa35f141a00fa9f3eec3 100644 | ||
--- a/dist/index.d.ts | ||
+++ b/dist/index.d.ts | ||
@@ -1,34 +1,37 @@ | ||
-import { Plugin } from 'vite'; | ||
-import { ZlibOptions } from 'zlib'; | ||
-import { Plugin as Plugin$1 } from 'rollup'; | ||
+import { Plugin } from "vite"; | ||
+import { ZlibOptions } from "zlib"; | ||
+import { Plugin as Plugin$1 } from "rollup"; | ||
|
||
-type AnalyzerMode = 'static' | 'json' | 'server'; | ||
-type DefaultSizes = 'stat' | 'parsed' | 'gzip'; | ||
+type AnalyzerMode = "static" | "json" | "server"; | ||
+type DefaultSizes = "stat" | "parsed" | "gzip"; | ||
interface BasicAnalyzerPluginOptions { | ||
- summary?: boolean; | ||
- analyzerMode?: AnalyzerMode; | ||
- reportTitle?: string; | ||
- defaultSizes?: DefaultSizes; | ||
- gzipOptions?: ZlibOptions; | ||
+ summary?: boolean; | ||
+ analyzerMode?: AnalyzerMode; | ||
+ reportTitle?: string; | ||
+ defaultSizes?: DefaultSizes; | ||
+ gzipOptions?: ZlibOptions; | ||
} | ||
interface AnalyzerPluginOptionsWithServer extends BasicAnalyzerPluginOptions { | ||
- analyzerMode?: 'server'; | ||
- analyzerPort?: number | 'auto'; | ||
- openAnalyzer?: boolean; | ||
+ analyzerMode?: "server"; | ||
+ analyzerPort?: number | "auto"; | ||
+ openAnalyzer?: boolean; | ||
} | ||
interface AnalyzerPluginOptionsWithJson extends BasicAnalyzerPluginOptions { | ||
- analyzerMode?: 'json'; | ||
- fileName?: string; | ||
+ analyzerMode?: "json"; | ||
+ fileName?: string; | ||
} | ||
interface AnalyzerPluginOptionsWithStatic extends BasicAnalyzerPluginOptions { | ||
- analyzerMode?: 'static'; | ||
- analyzerPort?: number | 'auto'; | ||
- openAnalyzer?: boolean; | ||
- fileName?: string; | ||
+ analyzerMode?: "static"; | ||
+ analyzerPort?: number | "auto"; | ||
+ openAnalyzer?: boolean; | ||
+ fileName?: string; | ||
} | ||
-type AnalyzerPluginOptions = AnalyzerPluginOptionsWithServer | AnalyzerPluginOptionsWithStatic | AnalyzerPluginOptionsWithJson; | ||
+type AnalyzerPluginOptions = | ||
+ | AnalyzerPluginOptionsWithServer | ||
+ | AnalyzerPluginOptionsWithStatic | ||
+ | AnalyzerPluginOptionsWithJson; | ||
|
||
-declare function adapter(userPlugin: Plugin): Plugin$1<any>; | ||
+declare function adapter(userPlugin: Plugin): Plugin$1; | ||
|
||
declare function analyzer(opts?: AnalyzerPluginOptions): Plugin; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vite-bundle-analyzer
has arollup
very flexible peer dependency (2.X -> 4.X) but it actually requires 3.X at least because of a typescript type usage.Since we're using rollup 2.X in the repo (force by
rollup
), the alternative I took is to change the type usage (very simple change)