Support production profiling with React Developer Tools by JacobMGEvans · Pull Request #7737 · react/create-react-app

JacobMGEvans · 2019-09-26T02:13:07Z

Closes#7734.

Some of the process I went through to test the changes include the following:

I ran yarn test

After I ran yarn build

Following this, I ran yarn create-react-app my-app built a small (very small) app
to test the react devtools profiler and source code

**Source Code ClassName:**

…s and functionNames for human readbility

facebook-github-bot · 2019-09-26T02:13:16Z

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!

facebook-github-bot · 2019-09-26T03:20:00Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

ianschmitz · 2019-09-26T04:50:21Z

~~Looks like this needs to rebased on master.~~

Scratch that. Target branch is wrong that's why it's giving a strange diff.

ianschmitz

Can you revert yarn.lock.cached please?

JacobMGEvans · 2019-09-26T04:58:57Z

Do you mean to before I changed it and caused the merge conflict?

ianschmitz · 2019-09-26T05:00:43Z

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

JacobMGEvans · 2019-09-26T05:14:57Z

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.

I was able to revert the file to the master branch yarn.lock.cache to the previous state on commit
e0719e4c5f00d2870ae16a2b57a00c66941ea429

kentcdodds · 2019-09-26T15:17:31Z

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.

kentcdodds · 2019-09-26T22:37:32Z

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.

JacobMGEvans · 2019-09-26T22:38:16Z

What about yarn profile or yarn build --profile? Not sure I'm a fan of using an env variable here.

@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 😄

bvaughn · 2019-09-27T17:12:36Z

This is slick 👍

I also think yarn build --profile (or similar, command flag) would be preferable to an ENV var.

JacobMGEvans · 2019-09-28T02:40:46Z

This is slick 👍
I also think yarn build --profile (or similar, command flag) would be preferable to an ENV var.

I will work on figuring out how to accomplish it this way then.

I appreciate your input and suggestions @gaearon @bvaughn

kentcdodds · 2019-09-28T02:44:41Z

I think you should be able to get it with process.env.argv.includes('--profile')

JacobMGEvans · 2019-09-28T03:28:35Z

I think you should be able to get it with process.env.argv.includes('--profile')

Thank you very much for your help @kentcdodds 🎉 😃

- Per suggestion --profile flag used instead of env variable PROFILE_APP

ianschmitz

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 --profile flag. Use npm run build -- --profile or yarn build --profile to 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

bvaughn · 2019-09-29T16:27:29Z

Once this lands, let me know and I can add it to https://fb.me/react-profiling as well.

JacobMGEvans · 2019-09-30T02:35:44Z

@ianschmitz I added what you suggested for potential documentation to the section you mentioned

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 --profile flag. Use npm run build -- --profile or yarn build --profile to 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

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! 😄

kentcdodds

Super 👍

- 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.

iansu · 2019-10-03T04:24:26Z

@kentcdodds @bvaughn This change is released now in v3.2.0.

bvaughn · 2019-10-03T14:54:33Z

Thanks a bunch!

Docs have been updated:
http://fb.me/react-profiling#create-react-app-v32

JacobMGEvans added 2 commits September 25, 2019 20:01

Added the alias for profiling and output change to keep the className…
ee4af55
…s and functionNames for human readbility

defined isEnvProductionProfile with other isEnv checks
46cd4cd

JacobMGEvans mentioned this pull request Sep 26, 2019
Make it easier to properly profile with the React DevTools. #7734
Closed

kevin940726 reviewed Sep 26, 2019
View reviewed changes

Comment threadpackages/react-scripts/config/webpack.config.js Outdated

moved the keep_classnames and keep_fnames to terserOptions scope
48976fd

facebook-github-bot added the CLA Signed label Sep 26, 2019

JacobMGEvans marked this pull request as ready for review September 26, 2019 04:15

JacobMGEvans requested review from amyrlam, ianschmitz, iansu, mrmckeb and petetnt as code owners September 26, 2019 04:15

ianschmitz added the tag: new feature label Sep 26, 2019

ianschmitz added this to the 3.2 milestone Sep 26, 2019

ianschmitz changed the title ~~Feature profile issue#7734~~Support production profiling with React Developer ToolsSep 26, 2019

resolve merge conflict for yarn.lock.cache
3f3c3bc

ianschmitz changed the base branch from feature/templates to masterSeptember 26, 2019 04:56

ianschmitz suggested changes Sep 26, 2019
View reviewed changes

revert yarn.lock.cache to master yarn.lock.cache
e52c452
- git checkout origin/master -- packages/create-react-app/yarn.lock.cached

JacobMGEvans requested a review from ianschmitz September 26, 2019 05:22

petetnt reviewed Sep 26, 2019
View reviewed changes

Comment threadpackages/react-scripts/config/webpack.config.js Outdated

iansu self-assigned this Sep 26, 2019

Comment and Boolean Check
56d0841
- 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.

Replaced env with flag
dd71838
- Per suggestion --profile flag used instead of env variable PROFILE_APP

ianschmitz suggested changes Sep 28, 2019
View reviewed changes

documentation in available scripts section with suggested information
bc08ea6

kentcdodds reviewed Sep 30, 2019
View reviewed changes

Comment threaddocusaurus/docs/available-scripts.md Outdated

resolved a local git issue. Fixed documentation error.
6814e38

kentcdodds approved these changes Sep 30, 2019
View reviewed changes

iansu modified the milestones: 3.2, 3.1.3 Sep 30, 2019

iansu suggested changes Sep 30, 2019
View reviewed changes

Comment threaddocusaurus/docs/available-scripts.md Outdated

ianschmitz modified the milestones: 3.2, 3.3 Oct 1, 2019

moved documentation to suggested file
0684979
- 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.

ianschmitz approved these changes Oct 2, 2019
View reviewed changes

ianschmitz modified the milestones: 3.3, 3.2 Oct 2, 2019

iansu added 2 commits October 2, 2019 21:24

Update production-build.md
94d5839

Merge branch 'master' into feature-profile-issue#7734
b2589c0

iansu approved these changes Oct 3, 2019
View reviewed changes

iansu merged commit 88cf8cd into react:masterOct 3, 2019

lockBot locked and limited conversation to collaborators Oct 8, 2019

Uh oh!

Uh oh!

Conversation

JacobMGEvans commented Sep 26, 2019• edited by ianschmitz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Sep 26, 2019

Uh oh!

Uh oh!

facebook-github-bot commented Sep 26, 2019

Uh oh!

ianschmitz commented Sep 26, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianschmitz left a comment

Choose a reason for hiding this comment

Uh oh!

JacobMGEvans commented Sep 26, 2019

Uh oh!

ianschmitz commented Sep 26, 2019

Uh oh!

JacobMGEvans commented Sep 26, 2019

Uh oh!

Uh oh!

kentcdodds commented Sep 26, 2019

Uh oh!

kentcdodds commented Sep 26, 2019

Uh oh!

JacobMGEvans commented Sep 26, 2019

Uh oh!

bvaughn commented Sep 27, 2019

Uh oh!

JacobMGEvans commented Sep 28, 2019

Uh oh!

kentcdodds commented Sep 28, 2019

Uh oh!

JacobMGEvans commented Sep 28, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianschmitz left a comment

Choose a reason for hiding this comment

Uh oh!

bvaughn commented Sep 29, 2019

Uh oh!

JacobMGEvans commented Sep 30, 2019

Uh oh!

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iansu commented Oct 3, 2019

Uh oh!

bvaughn commented Oct 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

JacobMGEvans commented Sep 26, 2019•
edited by ianschmitz
Loading

ianschmitz commented Sep 26, 2019•
edited
Loading

JacobMGEvans commented Sep 28, 2019•
edited
Loading