Make it easier to properly profile with the React DevTools. #7734
Description
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 to resolve.alias:
'react-dom$': 'react-dom/profiling',
'scheduler/tracing': 'scheduler/tracing-profiling',And you have to add this to the Terser options:
keep_classnames: true,
keep_fnames: true,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: https://github.com/facebook/create-react-app/blob/0d1775e739b091affaf8b11a85aaa435fc53ee64/packages/react-scripts/config/webpack.config.js#L302-L306
- alias: {
+ alias: removeEmpty({
// Support React Native Web
// https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/
'react-native': 'react-native-web',
+ 'react-dom$': isEnvProductionProfile ? 'react-dom/profiling' : null,
+ 'scheduler/tracing': isEnvProductionProfile ? 'scheduler/tracing-profiling' : null,
+ }),
- },Where removeEmpty is something like (borrowed from webpack-config-utils):
function removeEmpty(input) {
const output = {}
Object.keys(input).forEach(key => {
const value = input[key]
if (typeof value !== 'undefined') {
output[key] = value
}
})
return output
}and isEnvProductionProfile is defined as: isEnvProduction && Boolean(process.env.PROFILE_APP).
We'd also add this right above: https://github.com/facebook/create-react-app/blob/0d1775e739b091affaf8b11a85aaa435fc53ee64/packages/react-scripts/config/webpack.config.js#L237
keep_classnames: isEnvProductionProfile,
keep_fnames: isEnvProductionProfile,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.
Activity
kentcdodds commentedon Sep 25, 2019
By the way, I don't really care exactly how this is implemented. If you don't like the environment variable/
removeEmptyfunction/etc, that's fine. I just want to make this easier.JacobMGEvans commentedon Sep 25, 2019
Are those steps to implementing this? Could we just make a PR then?
kentcdodds commentedon Sep 25, 2019
I think so!
jamesb3ll commentedon Sep 25, 2019
Instead of
removeEmptyfunction, spread operator could be used:JacobMGEvans commentedon Sep 26, 2019
Would any type safety be lost? Or would that short circuit be handling that logically equivalent?
JacobMGEvans commentedon Sep 26, 2019
I started to implement this here is the draft PR: #7737
Currently, I have an issue with the
keep_classnamesand I am guessingkeep_fnamesmight have some issues.Here is the current compiler error:
yarn run v1.17.3
$ cd packages/react-scripts && node bin/react-scripts.js build
Creating an optimized production build...
Failed to compile.
kevin940726 commentedon Sep 26, 2019
@JacobMGEvans I left the comment in the PR :)
JacobMGEvans commentedon Sep 26, 2019
@kevin940726 Pointing out I had the
keep_classnamesandkeep_fnamesinside of theoutput: { ... }rather than outside of it in the scope of theterserOptions: { ... }@kentcdodds works as you expected now, I just misread your direction for that part of the implementation.
kentcdodds commentedon Oct 3, 2019
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 😁
iansu commentedon Oct 3, 2019
@kentcdodds It's released now!