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

RFC: Initial outline for new devtools api #5462

Closed
wants to merge 1 commit into from

Conversation

jimfb
Copy link
Contributor

@jimfb jimfb commented Nov 12, 2015

Posting primarily to get feedback/thoughts/comments, and catch concerns early rather than late in the process.

The basic idea is to use a CQRS (or more specifically, Event Sourcing) pattern whereby the core will emit an event every time it does something (starts a render, ends a render, registers a dom-event-handler, resolves a ref, etc). The devtools can listen to each of these events, and derive a shadow copy of any state desired. This allows devtools to "follow along" with everything React is doing, and emit very information-full error messages, without us actually routing things through the core for the sole purpose of warnings.

Devtools are registered with React, which is global state, but is effectively what was happening before (except the current devtools uses monkeypatching, this is an explicit public API)

The "devmode" build of React (ie. the one that emits warnings) would simply be a copy of React bundled with a default devtool that registers its self with React, and keeps the necessary state to emit warnings. This means that the production and dev code paths become more similar, because the only difference is that in production emitEvent is a no-op.

This was built with lessons learned from other tools (hot-reloading, people wanting to customize the warnings module, etc) in mind. It allows other people to build custom devtools/warnings/etc. See #5306 for details on the objectives.

@jimfb
Copy link
Contributor Author

jimfb commented Nov 12, 2015

@sebmarkbage has context on this. cc @spicyj for reference.

@sophiebits
Copy link
Collaborator

cc @gaearon, #4593

@gaearon
Copy link
Collaborator

gaearon commented Nov 12, 2015

In general, I like this.

I have a question: how does this design deal with attaching mid-way? For example, say there's an async dev tool that only connects to a page after some initialization period. Or say it allows reconnecting. (I believe React DevTools currently let you do this?)

If a devtool derives all of its internal state from an event log, if it misses the start of the event log and connects asynchronously, it can't derive the state. However keeping a reference to the event log is also wasteful.

Am I overthinking this?

@gaearon
Copy link
Collaborator

gaearon commented Nov 12, 2015

Another question is whether the DevTools API is meant to be completely read-only.
An event log approach is read-only, but it doesn't seem to let you:

  • force re-render of an arbitrary instance (extremely useful to hot reloading)
  • prevent re-renders of an arbitrary instance (useful for a perf tool to find bottlenecks)
  • hook into render and lifecycle to catch errors (useful for RN-like error reporting with hot reloading)
  • tweak instance's props or state from the inspector (not sure it's useful given React's model though)

Forcing re-render is a big use case for me. (Especially so for the functional components that don't even have an instance to call forceUpdate() on.)

@jimfb
Copy link
Contributor Author

jimfb commented Nov 13, 2015

@gaearon
Obviously you should try to attach all devtools before first render. However, attaching mid-way is permitted, and the newly-attached devtool would miss the history. There are a couple of options here. The simplest is that the newly-attached devtool ignores events until it finds a "synchronizing event" such as a root-level call to React.render that it understands, and can start building internal state from that point forward (This strategy is stolen from the idea/way that compilers recover from invalid parse trees). With such an approach, once all the root components on a page have done a full re-render, the devtool is "caught up". We could force a rerender of all components on the page upon a devtool being added, or at the very least emit "welcome" event announcing that the new devtool has been attached and providing a basic synchronization event that lists the current React roots. So my answer would be: yes, still figuring out the details though. Do you have any thoughts on how best to do this?

Forcing rerender is easy - you will already know the React root's "container" node (from the previous render events your devtool witnessed) and you will already have the element that was rendered to that node (from that same previous render event), so you can just invoke React.render. Alternatively, you could store the instance that resulted and invoke forceUpdate().

Regarding read only: This API is intended to provide one side of the CQRS flow. Specifically, this API handles QUERY (all data that React provides will come in the form of these events). It does not provide the command side/flow, which is completely independent by definition of CQRS. For now, the command side would require utilizing the existing public API (or reaching into internal/private API) until such time as we can design+expose an intelligent set of commands for devtools.

@gaearon
Copy link
Collaborator

gaearon commented Nov 13, 2015

How do you plan to deal with functional components? They don't have (public) instances, but dev tools may need to re-render them or at least show them in a tree. Do they have IDs but not instances? Do dev tools receive internal instances?

@jimfb
Copy link
Contributor Author

jimfb commented Nov 13, 2015

@gaearon That question is slightly orthogonal to this API (it's a matter of what is doable in the React core, regardless of the communication mechanism). That said, functional components will not have internal instances and so there is no way to reference/rerender only a functional component. You would need to rerender the closest parent ancestor.

@gaearon
Copy link
Collaborator

gaearon commented Nov 13, 2015

I was curious what your thoughts on these are, as in an app made purely with functional components (extreme but entirely possible case), there isn't a single instance one could re-render. But I agree it's orthogonal so let's discuss later when there are more specifics.

@jimfb
Copy link
Contributor Author

jimfb commented Jan 14, 2016

Merged in #5590.

@jimfb jimfb closed this Jan 14, 2016
@oliverwoodings
Copy link

@jimfb sorry for the late comments on this. Unless I am mistaken, this api only works in dev right? But what if you want to have your devtools work in production for debugging purposes?

@oliverwoodings
Copy link

Example use case: I am working on an e2e testing library that would want to use this api for performing assertions, however due to the nature of e2e you don't want to restrict yourself to only testing dev builds

@jimfb
Copy link
Contributor Author

jimfb commented Jan 23, 2016

@oliverwoodings Correct, dev only.

FWIW, due to the nature of e2e tests you only want to test user visible behaviors. You shouldn't need a devtool (which is specifically for the purposes of peaking into internals) in your e2e tests, IMHO.

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.

5 participants