-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
Add Fiber Debugger #8033
Add Fiber Debugger #8033
Conversation
This lets us consume it from the debugger.
@@ -61,219 +62,231 @@ function flattenChildren(children : HostChildren<Instance | TextInstance>) { | |||
return flatArray; | |||
} | |||
|
|||
var NoopRenderer = ReactFiberReconciler({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just indentation changes + pass options.debugTool
through to the reconciler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is meant to be an alternative, and preferred, solution instead of the other comments.
The most important thing to me is that we can easily strip this out today because I don't want to have to skip through and worry about this when we micro-optimize.
It is also important that we don't expose this to the public renderer API so that others don't start building devtools with it, or that we start building public devtools that rely on the fiber structure.
It seems to me that the use of this tool means that you're inside the React debugging environment and therefore you have access to your own build.
So you can just create a global devtool module that can get a dynamic injection from anywhere. Just like ReactInstrumentation/ReactDebugTool does.
Then you hide that behind a __DEV__
or __REACT_DEV__
flag that we can filter out so it doesn't get packaged.
That way you don't have to expose a custom constructor for each renderer to invoke this. You don't have to pass it around anyway. You can just require it globally and the tool can just attach to the instrumentation.
scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void | ||
scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void, | ||
|
||
debugTool : DebugTool | null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the debug tool should be part of the host environment since we can use this with any host environment.
Adding things to this API should come with a very high bar since it makes it difficult to implement new renderers and it create a permanent type that we have to support.
It would be better if you passed this as a separate argument, separate from HostConfig.
E.g. just pass it as debugTool
parallel to config
in the places you need it.
} else { | ||
// Otherwise, complete the current work. | ||
return completeUnitOfWork(workInProgress); | ||
if (debugTool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should wrap all of these in __DEV__
before we have the infrastructure in place to statically DCE them.
0dd6cc8
to
4f0b8f0
Compare
@sebmarkbage Addressed. |
ok! |
* Build react-noop as a package This lets us consume it from the debugger. * Add instrumentation to Fiber * Check in Fiber Debugger
Fiber Debugger
This is a debugger handy for visualizing how Fiber works internally.
It is only meant to be used by React contributors, and not by React users.
It is likely that it might get broken at some point. If it's broken, ping Dan.
Running
First,
npm run build
in React root repo folder.Then
npm install
andnpm start
in this folder.Open
http://localhost:3000
in Chrome.Features
ReactNoop
renderer