fix(platform-browser): prevent duplicate stylesheets from being created (angular#52019)

mattlewis92 · atscott · commit 65786b2b96ba · 2023-10-05T14:06:56.000-07:00
Fixes a bug with REMOVE_STYLES_ON_COMPONENT_DESTROY when a component is destroyed and re-created, its previous stylesheets will not be re-used and instead a new stylesheet will still be created each time. PR Close angular#52019
diff --git a/packages/platform-browser/src/dom/shared_styles_host.ts b/packages/platform-browser/src/dom/shared_styles_host.ts
@@ -16,7 +16,7 @@ const APP_ID_ATTRIBUTE_NAME = 'ng-app-id';
 export class SharedStylesHost implements OnDestroy {
   // Maps all registered host nodes to a list of style nodes that have been added to the host node.
   private readonly styleRef = new Map < string /** Style string */, {
-    elements: HTMLStyleElement[];
+    elements: Map</** Host */ Node, /** Style Node */ HTMLStyleElement>;
     usage: number;
   }
   > ();
@@ -84,6 +84,9 @@ export class SharedStylesHost implements OnDestroy {
 
   removeHost(hostNode: Node): void {
     this.hostNodes.delete(hostNode);
+    for (const {elements} of this.styleRef.values()) {
+      elements.delete(hostNode);
+    }
   }
 
   private getAllStyles(): IterableIterator<string> {
@@ -125,11 +128,17 @@ export class SharedStylesHost implements OnDestroy {
     }
 
     const usage = nonNegativeNumber(delta);
-    map.set(style, {usage, elements: []});
+    map.set(style, {usage, elements: new Map()});
     return usage;
   }
 
-  private getStyleElement(host: Node, style: string): HTMLStyleElement {
+  private getStyleElement(
+      host: Node, style: string,
+      existingStyleElements: Map<Node, HTMLStyleElement>|undefined): HTMLStyleElement {
+    const existingStyleElement = existingStyleElements?.get(host);
+    if (existingStyleElement) {
+      return existingStyleElement;
+    }
     const styleNodesInDOM = this.styleNodesInDOM;
     const styleEl = styleNodesInDOM?.get(style);
     if (styleEl?.parentNode === host) {
@@ -162,23 +171,19 @@ export class SharedStylesHost implements OnDestroy {
   }
 
   private addStyleToHost(host: Node, style: string): void {
-    const styleEl = this.getStyleElement(host, style);
+    const styleRef = this.styleRef;
+    const styleResult = styleRef.get(style)!;  // This will always be defined in `changeUsageCount`
+    const styleEl = this.getStyleElement(host, style, styleResult.elements);
 
     host.appendChild(styleEl);
 
-    const styleRef = this.styleRef;
-    const styleResult = styleRef.get(style);
-    if (styleResult) {
-      if (styleResult.usage === 0) {
-        disableStylesheet(styleEl);
-      } else {
-        enableStylesheet(styleEl);
-      }
-
-      styleResult.elements.push(styleEl);
+    if (styleResult.usage === 0) {
+      disableStylesheet(styleEl);
     } else {
-      styleRef.set(style, {elements: [styleEl], usage: 1});
+      enableStylesheet(styleEl);
     }
+
+    styleResult.elements.set(host, styleEl);
   }
 
   private resetHostNodes(): void {
diff --git a/packages/platform-browser/test/dom/dom_renderer_spec.ts b/packages/platform-browser/test/dom/dom_renderer_spec.ts
@@ -262,6 +262,31 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
         expect(styles?.length).toBe(1);
         expect(styles?.[0].nativeElement.disabled).toBeTrue();
       });
+
+      it('should not add duplicate stylesheets', async () => {
+        const fixture = TestBed.createComponent(SomeAppForCleanUp);
+        const compInstance = fixture.componentInstance;
+        compInstance.showEmulatedComponents = false;
+        compInstance.componentTwoInstanceHidden = true;
+
+        fixture.detectChanges();
+        // verify style is in DOM
+        expect(await styleCount(fixture, '.none')).toBe(1);
+
+        // Remove a single instance of the component.
+        compInstance.componentOneInstanceHidden = true;
+        fixture.detectChanges();
+
+        // Verify style is still in DOM
+        expect(await styleCount(fixture, '.none')).toBe(1);
+
+        // Re-create the component
+        compInstance.componentOneInstanceHidden = false;
+        fixture.detectChanges();
+
+        // Verify there is only 1 style
+        expect(await styleCount(fixture, '.none')).toBe(1);
+      });
     });
   });
 }
diff --git a/packages/platform-browser/test/dom/shared_styles_host_spec.ts b/packages/platform-browser/test/dom/shared_styles_host_spec.ts
@@ -61,5 +61,14 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
       ssh.addHost(someHost);
       expect(someHost.innerHTML).toEqual('<style nonce="{% nonce %}">a {};</style>');
     });
+
+    it('should not add duplicate style nodes', () => {
+      ssh.addHost(someHost);
+      const styles = ['a {};'];
+      ssh.addStyles(styles);
+      ssh.disableStyles(styles);
+      ssh.addStyles(styles);
+      expect(someHost.innerHTML).toEqual('<style>a {};</style>');
+    });
   });
 }
