-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Enable minification in production builds #7748
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
base: main
Are you sure you want to change the base?
Conversation
Fixes jupyter#7736 - Changed minimize: false to minimize: true in webpack.prod.config.js - This should reduce asset size from ~63MB to ~17MB - Matches JupyterLab's optimized build approach
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.
- Reduces asset size from ~63MB to ~17MB (approximately 73% reduction)
- Improves page load times
- Reduces bandwidth usage
🎉
Faster pip/conda installations
Does it? It looks like the release artifacts actually ended up being larger than on main:
I wonder if there are some options for the minimizer/terser we should copy from JupyterLab (although I also think we should update those in JupyterLab as well): https://github.com/jupyterlab/jupyterlab/blob/21f9c73298d7a450dba9927d404c87aae3011f7a/jupyterlab/staging/webpack.prod.minimize.config.js#L21-L35
|
Ah, I see what happened - the minification worked on the JS files themselves, but the source maps ended up bigger which defeated the whole purpose. Makes sense that the total package size increased instead of decreased. |
|
I don't know which one would be optimal approach, this requires experimenting a bit and documenting the results. It feels like turning the comments off could help reduce the size further (if it is not a default already). I don't mind the release tarball increasing slightly if it means faster load time, but we need to have data to support it. |
Curious how you were able to get these numbers without testing? |
|
Also the latest commit on https://github.com/jupyter/notebook/actions/runs/18644190793
While with this PR the numbers are slightly bigger? https://github.com/jupyter/notebook/actions/runs/18612834945?pr=7748
|
|
@jtpio yes this is due to map files increasing in size, see the collapsed details in my comment. |
|
@jtpio You're right to call that out - those numbers were an estimate based on comparing to JupyterLab's asset sizes, not actual testing. I should have been clearer that it was a projection, not a measured result. |
Copied terser configuration from JupyterLab to optimize both JS and source maps. Unable to test locally due to lib0/TypeScript compatibility issues, relying on CI for validation.
|
Hi so I've updated the PR with the TerserPlugin configuration from JupyterLab as you suggested. |
|
@HarkiratS1511 if you face build issues feel free to ask on jupyter zulip chat or discourse maybe we could help out. What you describe could have happened if using say As for testing you can download the built artifact from CI it's the result of Check Release workflow on the summary apge e.g. for latest commit here https://github.com/jupyter/notebook/actions/runs/18709256519?pr=7748 |
|
I see the artifact size went up to 30.6 MB instead of down. |










Fixes #7736
Description
Enables minification in production webpack builds by changing
minimize: falsetominimize: trueinapp/webpack.prod.config.js.Problem
The published frontend assets were ~63MB compared to JupyterLab's ~17MB, despite using the same underlying technology. Investigation revealed that the production webpack configuration explicitly disabled minification.
Changes
minimize: falsetominimize: trueinapp/webpack.prod.config.js(line 18)Expected Impact
Testing
Unable to test locally due to development environment constraints. The change is straightforward (single boolean flag), and CI will validate the build. Ready for maintainer review.