fix(core): don't coerce all producers to consumers on liveness change (#56140)

alxhub · thePunderWoman · commit 616cdef4748d · 2024-05-30T17:44:56.000Z
When a consumer switches its liveness state, it gets added to / removed from the consumer list of all of its producers. This operation is transitive, so if its producer is *also* a consumer and *its* liveness state is switched, then the change is applied recursively. Note that this only matters *if* the producer is also a consumer. However, the logic in `producerAddLiveConsumer` / `producerRemoveLiveConsumerAtIndex` coerced the producer node into a producer & consumer node, which allocated extra arrays into the node structure that are never used. This didn't affect correctness, but increased the memory usage of plain signal nodes (which are just producers, never consumers). This fix changes the logic in those operations to simply check if a producer is also a consumer instead of coercing it into one. PR Close #56140
diff --git a/packages/core/primitives/signals/src/graph.ts b/packages/core/primitives/signals/src/graph.ts
@@ -439,8 +439,7 @@ function producerAddLiveConsumer(
   indexOfThis: number,
 ): number {
   assertProducerNode(node);
-  assertConsumerNode(node);
-  if (node.liveConsumerNode.length === 0) {
+  if (node.liveConsumerNode.length === 0 && isConsumerNode(node)) {
     // When going from 0 to 1 live consumers, we become a live consumer to our producers.
     for (let i = 0; i < node.producerNode.length; i++) {
       node.producerIndexOfThis[i] = producerAddLiveConsumer(node.producerNode[i], node, i);
@@ -455,15 +454,14 @@ function producerAddLiveConsumer(
  */
 function producerRemoveLiveConsumerAtIndex(node: ReactiveNode, idx: number): void {
   assertProducerNode(node);
-  assertConsumerNode(node);
 
   if (typeof ngDevMode !== 'undefined' && ngDevMode && idx >= node.liveConsumerNode.length) {
     throw new Error(
       `Assertion error: active consumer index ${idx} is out of bounds of ${node.liveConsumerNode.length} consumers)`,
     );
   }
 
-  if (node.liveConsumerNode.length === 1) {
+  if (node.liveConsumerNode.length === 1 && isConsumerNode(node)) {
     // When removing the last live consumer, we will no longer be live. We need to remove
     // ourselves from our producers' tracking (which may cause consumer-producers to lose
     // liveness as well).
@@ -506,3 +504,7 @@ function assertProducerNode(node: ReactiveNode): asserts node is ProducerNode {
   node.liveConsumerNode ??= [];
   node.liveConsumerIndexOfThis ??= [];
 }
+
+function isConsumerNode(node: ReactiveNode): node is ConsumerNode {
+  return node.producerNode !== undefined;
+}
