Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReactDebugToolEventForwarderDevTool #6718

Conversation

troydemonbreun
Copy link
Contributor

Per discussion with @jimfb on 6398

@troydemonbreun
Copy link
Contributor Author

@jimfb I did not forward the *Flush and *Timer events in ReactDebugTool, should I do those as well?

@gaearon
Copy link
Collaborator

gaearon commented May 6, 2016

I think @sebmarkbage might have an opinion on this. Do we want all those events on both devtool integration points? Doesn't this increase (not yet public but eventually public) API surface quite a bit?

@jimfb
Copy link
Contributor

jimfb commented May 6, 2016

I'm also curious to hear what @sebmarkbage thinks.

I really don't have a strong opinion here. Just seems like the React events are among the most critical events, so it would make sense that each renderer would supply them to the devtools as part of the same stream/registration process.

This also avoids an edge case where someone writes a renderer that happens to use an event name which React subsequently chooses to use. Any devtool listening to both React and MyReactRenderer would break. However, if the renderer is responsible for forwarding the event, it means that renderer can rename/modify/appendInfo as appropriate for the given event.

@troydemonbreun
Copy link
Contributor Author

@jimfb Any interest from the team on this?

@gaearon
Copy link
Collaborator

gaearon commented May 25, 2016

@troydemonbreun

Sorry we didn’t come back to this. I believe @spicyj needed something similar in #6765 but went with a simpler option of adding any DOM devtools to ReactDebugTool automatically.

So this implementation appears no longer necessary, but an equivalent got merged. Thank you for the effort though!

@gaearon gaearon closed this May 25, 2016
@troydemonbreun troydemonbreun deleted the r-event-forwarder-devtool branch June 28, 2016 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants