[code-infra] Skip error minification inside dev-only branches#1069
[code-infra] Skip error minification inside dev-only branches#1069
Conversation
- Errors inside `if (process.env.NODE_ENV !== 'production')` blocks are already stripped in production, so minifying them is unnecessary - Avoids allocating error codes and injecting runtime imports for dead code
✅ Deploy Preview for mui-internal ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Janpot
added
the
scope: code-infra
Bundle size report
Check out the code infra dashboard for more information about this PR. |
oliviertassinari
changed the title
| output: readOutputFixtureSync('custom-runtime-imports-recursive', 'output.js'), | ||
| }, | ||
| { | ||
| title: 'skips errors inside dev-only branches', |
There was a problem hiding this comment.
How about we implement the opposite behavior? We throw if an error is thrown in an dev-only branch, unless minify-error-disabled is set?
The idea would be that we should almost never change the control flow between environments. If we start to allow this pattern, people might face bugs that only happen in production.
There was a problem hiding this comment.
I'm thinking we could make a lint rule for this specific pattern. Linting and minification are two different things. If possible would like to avoid encoding hidden linting rules in a process that's meant for something entirely different.
There was a problem hiding this comment.
Linting is indeed the correct place to have the logic I'm proposing, but it's also more Ci time, more work to maintain, I saw this as a quick win 😁. But ok fair enough. We can have both.
Janpot
force-pushed
the
minify-errors-skip-dev-branches
branch
from
953e2d8 to
0866b68
Compare
oliviertassinari
added
the
type: enhancement
4 participants
Summary
if (process.env.NODE_ENV !== 'production')blocks are already stripped in production, so minifying them is unnecessaryTesting in mui/base-ui#3869