Conversation
marcaaron
left a comment
marcaaron
left a comment
There was a problem hiding this comment.
Looks good just a couple comments. Nice tool.
wdyr.js
Outdated
| include: [ | ||
| /^Avatar/, | ||
| /^ReportActionItem/, | ||
| /^ReportActionItemSingle/, |
There was a problem hiding this comment.
Should we make it so all things are included by default? And maybe comment out this include/exclude section as optional?
There was a problem hiding this comment.
There is A LOT of noise when tracking every component and I'm not sure that opening my console to 100+ notifications about unrelated components would be that useful.
I think that tracking relevant components might be better.
There was a problem hiding this comment.
I understand, but also the feature is opt-in. So maybe the default would be to show everything unless otherwise specified. It's not difficult to filter for the exact component by filtering in the JS console. Mostly bringing this up because I have used this tool before and don't usually futz with the configs too much.
There was a problem hiding this comment.
Maybe as a compromise we can just mention how to show everything in a commen? :)
There was a problem hiding this comment.
The original GH did specify:
Implement WDYR and only enable it on certain "problem components" like ReportActionItem, Avatar etc.
But I agree having a comment here about how to enable it for everything would be nice to have.
There was a problem hiding this comment.
Well, what can I say, my past and future selves don't always get along ! 😂
There was a problem hiding this comment.
Fair enough! I made changes to display everything by default and left the comment on how to include/exclude certain components only.
|
also this can probably be No QA instead of Internal QA. |
marcaaron
changed the title
marcaaron
changed the title
|
Updated! |
wdyr.js
Outdated
| include: [ | ||
| /^Avatar/, | ||
| /^ReportActionItem/, | ||
| /^ReportActionItemSingle/, |
There was a problem hiding this comment.
The original GH did specify:
Implement WDYR and only enable it on certain "problem components" like ReportActionItem, Avatar etc.
But I agree having a comment here about how to enable it for everything would be nice to have.
|
Updated! |
| const whyDidYouRender = require('@welldone-software/why-did-you-render'); | ||
| whyDidYouRender(React, { | ||
| // Tracks all components by default | ||
| trackAllPureComponents: true, |
There was a problem hiding this comment.
Should this be false (the default)? PureComponent seems like the least likely to be re-rendering for bad reasons?
There was a problem hiding this comment.
After doing more research and seeing this comment from one of the creators, it seems that tracking pure components by default is a better idea, as they might control big render trees.
So I will keep that as the default, but I also included a way to enable tracking in all components, as well as examples to enable tracking by component displayName.
Screen must be excluded when tracking all components to avoid the following error that crashes the app:
Uncaught Error: A navigator can only contain 'Screen', 'Group' or 'React.Fragment' as its direct children (found 'WDYRFunctionalComponent' for the screen 'Home'). To render this component in the navigator, pass it in the 'component' prop to 'Screen'.
There was a problem hiding this comment.
Ok that works. Weird about the Screen component. Must have used this last before we added react navigation.
luacmartins
force-pushed
the
cmartins-wdyr
branch
from
455b30a to
901acd2
Compare
|
Updated! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging in version: 1.0.82-8🚀
|
|
🚀 Deployed to production by @francoisl in version: 1.0.83-1 🚀
|
4 participants
cc @marcaaron @chiragsalian
Details
Implements Why Did You Render to help track and avoid unnecessary re-renders. Once enabled, it will track re-renders of
Avatar,ReportActionItemandReportActionItemSingle. More components can be added inwdyr.js.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/165181
Tests
USE_WDYR=truein your.envfile.npm run webQA Steps
Internal QA
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android