-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update minification and sourcemap settings on CI builds for sentry #19583
Conversation
argv.mode is *always* set to production by 'yarn build' which is all that's called by CI
4dc9058
to
ced0dd7
Compare
Looks sensible. I can't see any reason un-minified sources would be useful if we ship the app with source maps anyway, whether or not we publish them to sentry - would be good to double check from others. |
I realised that the problem is actually the embedded source is huge, so trying with |
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.
otherwise this seems fine - developer machines won't explode, and seems to result in better bundles for end users. Assuming Sentry works with this, let's do it.
webpack.config.js
Outdated
const devMode = nodeEnv !== 'production'; | ||
const useHMR = process.env.CSS_HOT_RELOAD === '1' && devMode; | ||
const fullPageErrors = process.env.FULL_PAGE_ERRORS === '1' && devMode; | ||
const enableMinification = argv.mode === "production"; |
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.
I'd prefer if we instead used !devMode
or otherwise relied upon nodeEnv
as a variable
With previous settings, our JS files for develop are so large that sentry's webserver rejects the upload.
In this change -
eval-source-map
tosource-map
to move the embedded source out of the .js payload and into .js.map filesThis step fixed the issue for me locally at least by reducing the size of
init.js
from ~25MB to a 2MB js + 10MB .mapThis PR also includes a careful refactor of the loading of build config depending on the args and environment, its best to follow this commit by commit.
This PR currently has no changelog labels, so will not be included in changelogs.
A reviewer can add one of:
T-Deprecation
,T-Enhancement
,T-Defect
,T-Task
to indicate what type of change this is, or addType: [enhancement/defect/task]
to the description and I'll add them for you.