-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Make it easier to properly profile with the React DevTools. #7734
Comments
By the way, I don't really care exactly how this is implemented. If you don't like the environment variable/ |
Are those steps to implementing this? Could we just make a PR then? |
I think so! |
Instead of
|
Would any type safety be lost? Or would that short circuit be handling that logically equivalent? |
I started to implement this here is the draft PR: #7737 Failed to minify the bundle. Error: static/js/main.b21b40a1.chunk.js from Terser
`keep_classnames` is not a supported option
at /Users/jacobevans/personalProjects/create-react-app/packages/react-scripts/scripts/build.js:176:23
at finalCallback (/Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compiler.js:257:39)
at /Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compiler.js:273:13
at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
at AsyncSeriesHook.lazyCompileHook (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/Hook.js:154:20)
at onCompiled (/Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compiler.js:271:21)
at /Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compiler.js:681:15
at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
at AsyncSeriesHook.lazyCompileHook (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/Hook.js:154:20)
at /Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compiler.js:678:31
at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
at AsyncSeriesHook.lazyCompileHook (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/Hook.js:154:20)
at /Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compilation.js:1423:35
at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
at AsyncSeriesHook.lazyCompileHook (/Users/jacobevans/personalProjects/create-react-app/node_modules/tapable/lib/Hook.js:154:20)
at /Users/jacobevans/personalProjects/create-react-app/node_modules/webpack/lib/Compilation.js:1414:32
Read more here: https://bit.ly/CRA-build-minify |
@JacobMGEvans I left the comment in the PR :) |
@kevin940726 Pointing out I had the @kentcdodds works as you expected now, I just misread your direction for that part of the implementation. |
Huzzah!!!! Thanks @iansu! And well done @JacobMGEvans 👏 I understand that my needs are not your deadlines, but if this could be released for my workshop by next Monday that would be awesome 😁 |
@kentcdodds It's released now! |
I wrote a blog post about how to properly profile a React App for Performance and I use a
react-scripts
app to demonstrate how to do it. There are a few steps in there that I'd love to make easier.Specifically, you have to go into
./node_modules/react-scripts/config/webpack.config.js
to update the webpack config to add this toresolve.alias
:And you have to add this to the Terser options:
It's a bit of an annoying process and I think we could make it a lot easier (and hopefully encourage people to profile their apps in the process).
Describe the solution you'd like
We'd make a change here:
create-react-app/packages/react-scripts/config/webpack.config.js
Lines 302 to 306 in 0d1775e
Where
removeEmpty
is something like (borrowed from webpack-config-utils):and
isEnvProductionProfile
is defined as:isEnvProduction && Boolean(process.env.PROFILE_APP)
.We'd also add this right above:
create-react-app/packages/react-scripts/config/webpack.config.js
Line 237 in 0d1775e
Describe alternatives you've considered
Currently the alternative is to change it manually (and if you deploy from your local build then you need to remember to remove your edits).
Additional context
I'm bringing this up because I'd love to update my blog post to simply be: "Now, run the build in profiling mode with
npx cross-env PROFILE_APP=true npm run build
." That would be awesome. And it would make every-day profiling much simpler as well.The text was updated successfully, but these errors were encountered: