refactor(core): implement experimental getSignalGraph debug API #57074
Conversation
dgp1130
left a comment
There was a problem hiding this comment.
Thanks for putting this together! I really like the approach you came up with here and how it uses a Map. I have a number of small readability suggestions, but the main thing is probably to make sure we're fully defining and thinking through the nodes and edges data structure we're creating here.
ee728e7 to
87b020d
Compare
| assertLView(lView); | ||
| const templateLView = lView[tNode.index]; | ||
| if (templateLView) { | ||
| const templateConsumer = templateLView[REACTIVE_TEMPLATE_CONSUMER]; |
There was a problem hiding this comment.
Per @alxhub, there may be multiple template consumers for a component (one per view), so we need to keep those as well.
It seems that for the current moment we generally only use a single template effect per-component, but a template effect per-view is the direction we'd like to move for signal components and want to make sure DevTools is compatible with that.
There was a problem hiding this comment.
@alxhub, we discussed this issues a little while back, but it seems like Angular still uses only one consumer per component as we don't yet have different consumers per-view? @AleksanderBodurri and I were thinking it would make sense to stick with the current implementation for now and maybe revisit the one-consumer-per-view issue as we get closer to true @Component({ signal: true })? Is that the right approach or is there more we can/should do today in this space?
6019096 to
0a18702
Compare
dgp1130
left a comment
There was a problem hiding this comment.
This is looking great @AleksanderBodurri! I don't have any major concerns, just a handful of readability improvements and some file suggestions.
I'll defer to @alxhub and other FW folks on the nuances of DI usage though.
| assertLView(lView); | ||
| const templateLView = lView[tNode.index]; | ||
| if (templateLView) { | ||
| const templateConsumer = templateLView[REACTIVE_TEMPLATE_CONSUMER]; |
There was a problem hiding this comment.
@alxhub, we discussed this issues a little while back, but it seems like Angular still uses only one consumer per component as we don't yet have different consumers per-view? @AleksanderBodurri and I were thinking it would make sense to stick with the current implementation for now and maybe revisit the one-consumer-per-view issue as we get closer to true @Component({ signal: true })? Is that the right approach or is there more we can/should do today in this space?
a35e3d2 to
9a8e66f
Compare
Creates a debug api that returns an arrays of nodes and edges that represents a signal graph in the context of a particular injector. Starts by discovering the consumer nodes for each injector, and then traverses their dependencies to discover each producer.
9a8e66f to
a681d21
Compare
|
CARETAKER NOTE: g3 tests are passing with a single flake which can be ignored. http://test/OCL:704492061:BASE:704783405:1733858591915:66403176 |
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken |
|
This PR was merged into the repository by commit 33167d9. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…lar#57074) Creates a debug api that returns an arrays of nodes and edges that represents a signal graph in the context of a particular injector. Starts by discovering the consumer nodes for each injector, and then traverses their dependencies to discover each producer. PR Close angular#57074
Depends on #57073
Creates a debug api that returns an arrays of nodes and edges that represents a signal graph in the context of a particular injector.
Starts by discovering the consumer nodes for each injector, and then traverses their dependencies to discover each producer.