refactor(core): implement experimental getSignalGraph debug API #57074
refactor(core): implement experimental getSignalGraph debug API #57074AleksanderBodurri wants to merge 1 commit intoangular:mainfrom
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?
9d40359 to
252597c
Compare
There was a problem hiding this comment.
Does this require a separate PR since it's in core/primitives? @alxhub
There was a problem hiding this comment.
Yes it would. Let's do this step as a cleanup maybe? We can rely on the property sniffing for now.
There was a problem hiding this comment.
Sounds good, removed the modification to this file and implemented property sniffing approach temporarily.
There was a problem hiding this comment.
Yes it would. Let's do this step as a cleanup maybe? We can rely on the property sniffing for now.
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.
|
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. |
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.