refactor(devtools): adds router graph implementation and demo

[sumitarora]

Follow-up PR for #58086 & #58199.

Adds below features to Angular DevTools to display router graph:

• Add a new router example in the Angular DevTools demo application.
• Implement the router graph in the Angular DevTools to view the routes that are loaded in the application.

[dgp1130]

Thanks for putting this together @sumitarora!

I mostly left a bunch of minor readability improvements. I'll have to defer to @AleksanderBodurri on the DI and d3 stuff, as I'm not super familiar with those pieces and can only leave superficial suggestions.

The biggest missing piece to me is testing. I'd love to test some tests covering the newly added code to the extent we can. That would go a long way to maintaining functionality long term.

[dgp1130]

Suggestion: Avoid any where possible. I get that there's a bunch of pre-existing any types here and it doesn't look like we have an @angular/router dependency here (should we have one?). But anything we can do to avoid proliferating more any types is probably worth the effort, even if we ignore the existing ones.

[dgp1130]

Bug: What if the viewChild elements change? Should we recreate the RouterTreeVisualizer?

Ideally I think routerTreeVisualizer would be a computed(() => new RouterTreeVisualizer(...)), but I think the need to call .cleanup() makes that tricky. Maybe it should just be an effect dependent on viewChild signals?

This is a bit of an academic concern since those viewChild elements are unlikely to change, so up to you if this is really important to fix or not.

[dgp1130]

Consider: Since we only snap to root on the first render, could we use something like afterNextRender(() => this.routerTreeVisualizer.snapToRoot(...); in the constructor and then leave it out of renderGraph?

I'm thinking that getting rid of the parameter might make it a little easier to turn this into an effect which is automatically scheduled rather than manually managed.

[zygarios]

Pls, add option to set router tree vertical, not only horizontal

[dgp1130]

Thanks for these changes! It's looking a lot better already.

Is there anything we can do to write tests for some of the new additions? Maybe testing the new functionality in client-event-subscribers or router-tree.component / router-tree-visualizer?

I get that d3 probably isn't super conducive to unit testing, but even just making sure the right content was loaded and rendered at all goes a long way, even if we can't easily assert the exact layout of the graph.

[dgp1130]

Suggestion: I think find would be more appropriate than filter here. Also should we check that we actually found a Router instance? Not everyone will have @angular/router installed.

[dgp1130]

Consider: I think this can be marked protected?

[dgp1130]

Consider: I think this can be protected too?

[dgp1130]

Suggestion: Prefer boolean here as well (or just infer from false).

[dgp1130]

Question: What about rendering multiple routes?

[dgp1130]

Added a handful of minor suggestions, but nothing too major.

[dgp1130]

Consider: We can simplify this with a spread operator: serializedProviderRecords.push(...nonMultiRecords);

[dgp1130]

Nit: Unnecessary trailing space.

[dgp1130]

Typo: DEFAULT_FILTER

[dgp1130]

But I don't think ^ works after a .. What does "start of line" mean after a different character for a non-multiline regex?

IIUC:

• .* would match anything.
• ^. would match "starts a line with one character".
• .^ would match "one character followed by the start of a line" which feels like it would be never? Playing around with this myself, I can't get it to match any string.

[dgp1130]

Consider: The default case does new RegExp(DEFAULT_FILTER /* already a RegExp */). This seems to work, but maybe we should move the new RegExp into the non-default part of this condition to avoid duplicating the regex?

[AleksanderBodurri]

nit: This seems very similar to getInjectorProvidersCallback. Can we consolidate the logic (we can use this helper function in the getInjectorProvidersCallback logic).

[pkozlowski-opensource]

Adding cleanup label as it needs rebase

[sumitarora]

@pkozlowski-opensource rebased and fixed conflicts.

[pkozlowski-opensource]

This PR was merged into the repository by commit 42b1662.

The changes were merged into the following branches: main

[angular-automatic-lock-bot]

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.}