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

Provide info about component tree to devtools #6549

Merged
merged 20 commits into from Apr 26, 2016

Conversation

Projects
None yet
4 participants
@gaearon
Member

gaearon commented Apr 19, 2016

This is a work in progress that supersedes #6486 and #6488.

Unlike them, it doesn’t use an ID map and instead lets the core emit events as they happen.
(I should’ve started with this approach, it’s way simpler!)

I revert #6488 here because it is no longer necessary but can do this as a separate PR as well.

Things to do:

  • Update and unmount paths
  • Support and test server rendering
@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 20, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 20, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 20, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 20, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 20, 2016

@gaearon updated the pull request.

@gaearon gaearon changed the title from [WIP] Provide info about component tree to devtools to Provide info about component tree to devtools Apr 20, 2016

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 20, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 21, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 21, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 21, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 21, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 21, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 21, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 21, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 21, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 21, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 21, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 21, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 21, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 22, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 22, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 22, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 22, 2016

@gaearon updated the pull request.

@gaearon gaearon referenced this pull request Apr 23, 2016

Merged

Add new ReactPerf #6046

21 of 30 tasks complete
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon
Member

gaearon commented Apr 23, 2016

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 23, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 23, 2016

@gaearon updated the pull request.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Apr 23, 2016

Member

Awesome. I like this a lot. Let's just see what we can do about avoid the top level wrapper and if we really need the notion of "isComposite".

Member

sebmarkbage commented Apr 23, 2016

Awesome. I like this a lot. Let's just see what we can do about avoid the top level wrapper and if we really need the notion of "isComposite".

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 24, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 24, 2016

@gaearon updated the pull request.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Apr 24, 2016

Member

@sebmarkbage I removed isComposite and tried my best to hide TopLevelWrapper. I also noticed that I was cleaning up the information about unmounted instances too early, as I would need it in ReactPerf when the batch gets closed for analysis. This is why I tweaked it to only purge information on an explicit purgeUnmountedComponents() call, and in the result I made server rendering work the same way, and removed the need for passing native containers. Let me know if I missed some reason why they are important, but traversing the tree representation inside the devtool seems just as good to me, and makes the code clearer. I think this is ready now!

Member

gaearon commented Apr 24, 2016

@sebmarkbage I removed isComposite and tried my best to hide TopLevelWrapper. I also noticed that I was cleaning up the information about unmounted instances too early, as I would need it in ReactPerf when the batch gets closed for analysis. This is why I tweaked it to only purge information on an explicit purgeUnmountedComponents() call, and in the result I made server rendering work the same way, and removed the need for passing native containers. Let me know if I missed some reason why they are important, but traversing the tree representation inside the devtool seems just as good to me, and makes the code clearer. I think this is ready now!

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 24, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 24, 2016

@gaearon updated the pull request.

@iamdustan iamdustan referenced this pull request Apr 24, 2016

Open

DevTool hides children #15

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 24, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 24, 2016

@gaearon updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 24, 2016

@gaearon updated the pull request.

facebook-github-bot commented Apr 24, 2016

@gaearon updated the pull request.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Apr 25, 2016

Member

Ping @sebmarkbage, I would like to get this in asap because I’m stacking other work on top of this.

Member

gaearon commented Apr 25, 2016

Ping @sebmarkbage, I would like to get this in asap because I’m stacking other work on top of this.

@gaearon gaearon referenced this pull request Apr 25, 2016

Closed

Future of React Transform #90

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Apr 25, 2016

Member

I'll accept to avoid rebase problems but would be good to avoid exposing onSetIsEmpty (btw, if anything I think it is better to call them by a simple verb like onEmpty) and putting top level logic in the DebugTool.

Member

sebmarkbage commented Apr 25, 2016

I'll accept to avoid rebase problems but would be good to avoid exposing onSetIsEmpty (btw, if anything I think it is better to call them by a simple verb like onEmpty) and putting top level logic in the DebugTool.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Apr 25, 2016

Member

I’ll see what I can do there. Thanks!

Member

gaearon commented Apr 25, 2016

I’ll see what I can do there. Thanks!

@gaearon gaearon added this to the 15.x milestone Apr 26, 2016

@gaearon gaearon merged commit 76a4c46 into facebook:master Apr 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gaearon gaearon deleted the gaearon:instrumentation-new branch Apr 26, 2016

zpao added a commit that referenced this pull request May 10, 2016

Merge pull request #6549 from gaearon/instrumentation-new
Provide info about component tree to devtools
(cherry picked from commit 76a4c46)

@zpao zpao modified the milestones: 15.1.0, 15.y.0 May 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment