DevTools DI Visualization Features by AleksanderBodurri · Pull Request #51719 · angular/angular

AleksanderBodurri · 2023-09-10T03:37:32Z

See individual commits.

Note that the majority of the LOC additions come from mock data in unit tests.

yekanchi · 2023-09-10T12:12:17Z

any screenshot?

dgp1130

Thanks for putting this all together! Definitely a lot of d3 complexity here. I'm only about halfway through right now. Most of it are nitpicks / random code quality suggestions. Feel free to push back on any of these, I don't think any are blocking this PR.

The biggest question I have is what the impact of the @angular/core fix is on the versioning of the release. Does this mean we need the latest patch version to do any DI debugging? Is it fixing a specific bug in a specific situation or is this more load-bearing?

I would love to get a review from someone who understands more about DI. I can't provide a whole lot of feedback outside of general observations. Maybe Andrew Kushnir could take a look whenever you think it's ready?

dgp1130 · 2023-09-12T00:49:14Z

Consider: Are there any small utilities we could come up with to reduce the boilerplate and help communicate the intent of this code? d.parent and !d.parent are probably straightforward enough, but I wonder if d.data?.children?.length could be called hasChildren(d) or something like that? This would similarly help reduce the noise and draw more attention to a leading ! which is the important part of those expressions.

One other approach to consider (I don't really know if this makes much sense) is to better define the names of each of these cases like:

node.append('text').attr('dx', (d: any) => getDelta(d, { noParentNoChildren: 15, noParentHasChildren: 8, hasParentNoChildren: 15, hasParentHasChildren: -15, }));

dgp1130

Ok, finally got through the rest of it. Again, it's mostly pretty minor nitpicks and quality suggestions, nothing very critical. A couple comments are probably related to the fact that the PR is still in draft state and not fully complete.

dgp1130 · 2023-09-13T21:03:09Z

Nit: Indentation. We're inconsistently using 2 or 4 spaces.

dgp1130 · 2023-09-13T21:10:27Z

Suggestion: This is a big file and selectors like :host and ::ng-deep seem to repeat a lot. I suggest either grouping all the common selectors together or sectioning them into related styles. For example, .deps, .no-deps, :host-context(.dark-theme) { .deps { } } can be reasonably grouped together, even if other dark theme stuff exists elsewhere.

dgp1130 · 2023-09-13T21:21:32Z

Actually the biggest thing is probably the lack of tests. Testing the d3 stuff might be difficult and arguably not worth it, but we could certainly test a lot of the injection tree transformations and component behavior outside of rendering.

jessicajaniuk

reviewed-for: fw-core

dgp1130

Everything looks promising, so I'm very excited to this go in! I'll approve now just so this isn't blocked while I'm OOO, but take your time on the implementation. Definitely looking forward to seeing this land!

jessicajaniuk · 2023-09-27T12:49:42Z

@AleksanderBodurri Looks like this still has some CI issues.

AleksanderBodurri · 2023-10-02T16:44:59Z

Will do a bit more cleanup here related to @dgp1130 's feedback before we merge

AndrewKushnir · 2023-10-09T17:00:53Z

@AleksanderBodurri it looks like there is a legit error in core acceptance tests, see this CI job output. Could you please take a look when you get a chance?

AndrewKushnir · 2023-10-09T19:45:41Z

Update: this PR needs cleanup: there is a legit case were TNode might be null, but we don't account for this in the code (this triggers CI to fail). @AleksanderBodurri has additional info on this.

… getInjectorMetadata Previously getDependenciesForTokenInInjector was unable to determine which node on a view serviced a specific injection. Now it is able to filter out those injections that did not come from the specific node for the NodeInjector passed into it. Previously getInjectorMetadata was incorrectly looking up DOM elements for some directives (for example NgForOf) where an LContainer was created. Now the LContainer case is handled, and the non LContainer case uses `getFirstNativeNode` to more accurately get the element we want.

This commit introduces 2 new features into DevTools. Directive level dependency inspection: Users can now view which dependencies their directives have injected in the property viewer tab. This view displays not only the dependency but also the resolution path that was used to service the injection. Injector graph inspection: Users can now view a visualization of the element and environment hierarchies in their application. These trees are displayed separately but on the same page in the Injector Tree tab. User can click on individual injectors to view a list of all the providers configured in that injector, as well as highlight the resolution path from that injector to the root (with the corresponding environment injector connection highlighted as well).

…nctions Creates set of unit tests for each function in the data transformation pipeline that enables injector metadata to be visualized as d3 graphs.

AndrewKushnir · 2023-10-10T18:29:27Z

Presubmit.

AndrewKushnir · 2023-10-10T19:49:09Z

Caretaker note: presubmit and CI is "green", this PR is ready for merge.

angular-robot · 2023-10-10T20:10:52Z

This PR was merged into the repository by commit 8b91864.

…51719) This commit introduces 2 new features into DevTools. Directive level dependency inspection: Users can now view which dependencies their directives have injected in the property viewer tab. This view displays not only the dependency but also the resolution path that was used to service the injection. Injector graph inspection: Users can now view a visualization of the element and environment hierarchies in their application. These trees are displayed separately but on the same page in the Injector Tree tab. User can click on individual injectors to view a list of all the providers configured in that injector, as well as highlight the resolution path from that injector to the root (with the corresponding environment injector connection highlighted as well). PR Close #51719

…nctions (#51719) Creates set of unit tests for each function in the data transformation pipeline that enables injector metadata to be visualized as d3 graphs. PR Close #51719

haraldreingruber-dedalus · 2023-10-27T08:59:06Z

Hello. Thanks a lot for this contribution!

I would really like to test this on our project, as it might help us a lot for our refactoring.
I think the devtools in the chrome webstore wasn't updated in several months.
Can you help me figuring out the easiest way to run it locally?
I guess I have to update our project to one of the latest angular releases because of the backend part. Is the transpiled/compiled code of the devtools extension somewhere I can download and install locally?

Thanks a lot!

JeanMeche · 2023-10-27T09:00:56Z

The sources for the extensions are available on the repo. Here are the build instructions : https://github.com/angular/angular/blob/main/devtools/README.md

dgp1130 · 2023-10-27T14:19:19Z

@haraldreingruber-dedalus just FYI, this will be published alongside Angular v17 very shortly: https://angular.io/guide/releases#support-policy-and-schedule

Also this particular feature requires your app to be run on v17, so you'll need to upgrade first. You can test out 17.0.0-rc if you want to try the prerelease.

We'd definitely like to hear any feedback you have from trying it, I'm just letting you know what you're in for if you try to use this early.

haraldreingruber-dedalus · 2023-11-22T13:10:37Z

@AleksanderBodurri @dgp1130
I was finally able to update our app to Angular v17.
The injector-tree tab of the DevTools is working, and looks super nice so far 😎

Should I expect to see all entities of the DI container?
Could it be that @Injectable({providedIn: 'root'}) objects are missing, or do I misunderstand how it works?

Or maybe many important entities are missing because our project is split into several libraries?

Still, I think this feature is great for debugging and refactoring :)

angular-automatic-lock-bot · 2023-12-23T00:08:42Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

… getInjectorMetadata (angular#51719) Previously getDependenciesForTokenInInjector was unable to determine which node on a view serviced a specific injection. Now it is able to filter out those injections that did not come from the specific node for the NodeInjector passed into it. Previously getInjectorMetadata was incorrectly looking up DOM elements for some directives (for example NgForOf) where an LContainer was created. Now the LContainer case is handled, and the non LContainer case uses `getFirstNativeNode` to more accurately get the element we want. PR Close angular#51719

…ngular#51719) This commit introduces 2 new features into DevTools. Directive level dependency inspection: Users can now view which dependencies their directives have injected in the property viewer tab. This view displays not only the dependency but also the resolution path that was used to service the injection. Injector graph inspection: Users can now view a visualization of the element and environment hierarchies in their application. These trees are displayed separately but on the same page in the Injector Tree tab. User can click on individual injectors to view a list of all the providers configured in that injector, as well as highlight the resolution path from that injector to the root (with the corresponding environment injector connection highlighted as well). PR Close angular#51719

…nctions (angular#51719) Creates set of unit tests for each function in the data transformation pipeline that enables injector metadata to be visualized as d3 graphs. PR Close angular#51719

angular-robot Bot added the detected: feature PR contains a feature commit label Sep 10, 2023

AleksanderBodurri force-pushed the devtools-di-debugging branch 2 times, most recently from 9c92609 to ee0cff2 Compare September 10, 2023 04:38

KathrinShpigel mentioned this pull request Sep 11, 2023

Visualization of dependency injection #45748

Closed

dgp1130 requested changes Sep 12, 2023

View reviewed changes

AleksanderBodurri force-pushed the devtools-di-debugging branch from ee0cff2 to 5f65c8d Compare September 12, 2023 21:29

dgp1130 requested changes Sep 13, 2023

View reviewed changes

AleksanderBodurri marked this pull request as ready for review September 18, 2023 06:25

pullapprove Bot requested a review from jessicajaniuk September 18, 2023 06:25

pkozlowski-opensource added the area: devtools label Sep 18, 2023

ngbot Bot added this to the Backlog milestone Sep 18, 2023

jessicajaniuk approved these changes Sep 18, 2023

View reviewed changes

dgp1130 approved these changes Sep 18, 2023

View reviewed changes

AleksanderBodurri force-pushed the devtools-di-debugging branch from ce33cae to a542baa Compare September 25, 2023 16:57

angular-robot Bot requested a review from jessicajaniuk September 25, 2023 16:57

AleksanderBodurri force-pushed the devtools-di-debugging branch from a542baa to 176fd03 Compare October 2, 2023 16:41

AleksanderBodurri force-pushed the devtools-di-debugging branch 9 times, most recently from 97a87d7 to 477b6ea Compare October 6, 2023 16:24

AleksanderBodurri requested a review from AndrewKushnir October 6, 2023 16:50

AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 9, 2023

AleksanderBodurri force-pushed the devtools-di-debugging branch from e19afc5 to d114f27 Compare October 10, 2023 18:22

AleksanderBodurri added 3 commits October 10, 2023 14:23

test(devtools): create unit tests for injector tree transformation fu…

85374f7

…nctions Creates set of unit tests for each function in the data transformation pipeline that enables injector metadata to be visualized as d3 graphs.

AleksanderBodurri force-pushed the devtools-di-debugging branch from d114f27 to 85374f7 Compare October 10, 2023 18:23

AndrewKushnir approved these changes Oct 10, 2023

View reviewed changes

AndrewKushnir removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 10, 2023

jessicajaniuk approved these changes Oct 10, 2023

View reviewed changes

AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 10, 2023

angular-robot Bot closed this in 50ad074 Oct 10, 2023

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Dec 23, 2023

Conversation

AleksanderBodurri commented Sep 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yekanchi commented Sep 10, 2023

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgp1130 Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgp1130 Sep 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dgp1130 Sep 13, 2023

Choose a reason for hiding this comment

Uh oh!

dgp1130 commented Sep 13, 2023

Uh oh!

jessicajaniuk left a comment

Choose a reason for hiding this comment

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

jessicajaniuk commented Sep 27, 2023

Uh oh!

AleksanderBodurri commented Oct 2, 2023

Uh oh!

AndrewKushnir commented Oct 9, 2023

Uh oh!

AndrewKushnir commented Oct 9, 2023

Uh oh!

AndrewKushnir commented Oct 10, 2023

Uh oh!

AndrewKushnir commented Oct 10, 2023

Uh oh!

angular-robot Bot commented Oct 10, 2023

Uh oh!

haraldreingruber-dedalus commented Oct 27, 2023

Uh oh!

JeanMeche commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgp1130 commented Oct 27, 2023

Uh oh!

haraldreingruber-dedalus commented Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angular-automatic-lock-bot Bot commented Dec 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

AleksanderBodurri commented Sep 10, 2023 •

edited

Loading

JeanMeche commented Oct 27, 2023 •

edited

Loading

haraldreingruber-dedalus commented Nov 22, 2023 •

edited

Loading