[DevTools] Implemented re-render mechanism for React 16.9+ #17072
Conversation
hristo-kanchev
changed the title
| const fiber = findCurrentFiberUsingSlowPathById(id); | ||
|
|
||
| if (fiber !== null) { | ||
| if (typeof scheduleUpdate === 'function') { |
There was a problem hiding this comment.
Can't this line be merged with the line above?
There was a problem hiding this comment.
Yup! I've pushed the changes.
| @@ -125,6 +125,7 @@ export default class Agent extends EventEmitter<{| | |||
| bridge.addListener('getOwnersList', this.getOwnersList); | |||
| bridge.addListener('inspectElement', this.inspectElement); | |||
| bridge.addListener('logElementToConsole', this.logElementToConsole); | |||
| bridge.addListener('forceRerender', this.forceRerender); | |||
There was a problem hiding this comment.
super nit: Maybe just "forceRender" ? The "re" seems unnecessary.
|
I think we could improve the UX a bit though. When you click the button, there's no confirmation or signal that it's actually been clicked. Nothing happens. Maybe we need to make it stay highlighted for a second :think: or something. Thoughts? |
|
I guess I still don't quite understand the use case of this feature, so I'd like to hold off merging it for the time being until I understand it better, and we work out the UX issue. |
| @@ -2247,6 +2253,9 @@ export function attach( | |||
| // Can view component source location. | |||
| canViewSource, | |||
|
|
|||
| // Can force the component to re-render. | |||
| canForceRerender, | |||
There was a problem hiding this comment.
I think this should be conditional based on the element type. Right now it shows up for all types, including e.g. context providers and consumers, etc- re-rendering these doesn't make sense. Seems like this should be for class, function, forward ref, and maybe memo components types only?
|
Heya @bvaughn ! I'll address your comments and after that we can park it for the time being, until we get more feature requests like this one? What do you think? |
|
Sounds good, and thanks for being so understanding. The code approach here seems fine. I'm just not sure the feature itself makes total sense yet, until I better understand the use case :) |
|
We chatted on #16968 and agreed to close this for now. Thanks a bunch for the PR though! |
Solves: #16968
Description:
Added force re-render mechanism for components.
Support:
As @bvaughn suggested, we will only support React 16.9+
Versions that aren't supported won't show the
re-renderbutton in the UI.Tested on:
re-renderbutton in the UI.re-renderbutton in the UI and successfully dispatch an update to the supported components types.