Support production profiling with React Developer Tools #7737
Conversation
…s and functionNames for human readbility
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
JacobMGEvans
requested review from
amyrlam,
ianschmitz,
iansu,
mrmckeb and
petetnt
as code owners
|
Scratch that. Target branch is wrong that's why it's giving a strange diff. |
ianschmitz
changed the title
|
Do you mean to before I changed it and caused the merge conflict? |
If you revert it to whats in master then we shouldn't see any changes for that file, just the webpack config should show a diff. |
- git checkout origin/master -- packages/create-react-app/yarn.lock.cached
I was able to revert the file to the |
|
This is great 💯 |
- I clarified the comment and specified the use case - Changed the environment check to check for the specific true rather than the assumed primitive value as before.
|
I suggested the env variable because I didn't expect this to need a special script or flag. But I'm fine either way :) I just want it to be reasonably possible. |
@gaearon what could be some potential pitfalls or anything else you can think of, for not doing it this way? Just curious and trying to understand where you are coming from on this 😄 |
|
This is slick 👍 I also think |
|
I think you should be able to get it with |
Thank you very much for your help @kentcdodds 🎉 😃 |
- Per suggestion --profile flag used instead of env variable PROFILE_APP
There was a problem hiding this comment.
Looks good!
Let's add some documentation for this. We can probably add it to https://create-react-app.dev/docs/available-scripts for now. We can probably take some of the language from here:https://github.com/facebook/create-react-app/blob/cbad256a4aacfc3084be7ccf91aad87899c63564/docusaurus/docs/available-scripts.md
Maybe something like:
... production build section for more information.
React DOM automatically supports profiling in development mode for v16.5+, but since profiling adds some small additional overhead it is opt-in for production mode. You can opt-in by using the
--profileflag. Usenpm run build -- --profileoryarn build --profileto enable profiling in production mode. You can read more about profiling using the React DevTools here: https://reactjs.org/docs/optimizing-performance.html#profiling-components-with-the-devtools-profiler
|
Once this lands, let me know and I can add it to https://fb.me/react-profiling as well. |
|
@ianschmitz I added what you suggested for potential documentation to the section you mentioned
If anyone has other suggestions or ideas I am more than happy to proceed with further changes or discuss further changes. Thank you for the help and suggestions so far everyone! 😄 |
- Added a brief summary of profiling in available scripts section. The summary references the production-build document. Which is the file I moved the new documentation into under a new Header for production support.
|
@kentcdodds @bvaughn This change is released now in v3.2.0. |
|
Thanks a bunch! Docs have been updated: |
Closes #7734.
Some of the process I went through to test the changes include the following:
yarn testyarn buildyarn create-react-app my-appbuilt a small (very small) appto test the react devtools profiler and source code
**Source Code ClassName:**