perf(platform-browser): disable styles of removed components instead …

…of removing (#51808) This commit changes the behaviour of `REMOVE_STYLES_ON_COMPONENT_DESTROY`. Now, `style` nodes are disabled instead of removed from DOM. This causes the same runtime behaviour but avoids recomputations when the stylesheet is re-added when the component is re-created. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/css_style_sheet.h;l=266;drc=31fb07c05718d671d96c227855bfe97af9e3fb20 NB: This changes is being done following some performance bottlenecks observed in Phanteon and their own recommendations. Context: http://chat/room/AAAAxKxTk40/jaP6Lj6fhmQ/jaP6Lj6fhmQ https://crbug.com/1444522 http://b/289992821 PR Close #51808
angular · Sep 22, 2023 · 3c0577f · 3c0577f
1 parent 86e9146
commit 3c0577f
Show file tree

Hide file tree

Showing 12 changed files with 210 additions and 118 deletions.
diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
@@ -818,6 +818,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "documentElement"
   },
@@ -1247,6 +1250,9 @@
   {
     "name": "ngZoneApplicationErrorHandlerFactory"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json
@@ -881,6 +881,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "documentElement"
   },
@@ -1316,6 +1319,9 @@
   {
     "name": "noSideEffects"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
@@ -668,6 +668,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "empty"
   },
@@ -1055,6 +1058,9 @@
   {
     "name": "noSideEffects"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
@@ -905,6 +905,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "empty"
   },
@@ -1463,6 +1466,9 @@
   {
     "name": "noSideEffects"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
@@ -875,6 +875,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "empty"
   },
@@ -1427,6 +1430,9 @@
   {
     "name": "noSideEffects"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json
@@ -725,6 +725,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "empty"
   },
@@ -1127,6 +1130,9 @@
   {
     "name": "ngZoneApplicationErrorHandlerFactory"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -1118,6 +1118,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "dispatch"
   },
@@ -1727,6 +1730,9 @@
   {
     "name": "nodeChildrenAsMap"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json
@@ -599,6 +599,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "empty"
   },
@@ -929,6 +932,9 @@
   {
     "name": "ngZoneApplicationErrorHandlerFactory"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json
@@ -788,6 +788,9 @@
   {
     "name": "diPublicInInjector"
   },
+  {
+    "name": "disableStylesheet"
+  },
   {
     "name": "empty"
   },
@@ -1265,6 +1268,9 @@
   {
     "name": "noSideEffects"
   },
+  {
+    "name": "nonNegativeNumber"
+  },
   {
     "name": "nonNull"
   },

diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts
@@ -36,13 +36,13 @@ const REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT = true;
 
 /**
  * A [DI token](guide/glossary#di-token "DI token definition") that indicates whether styles
- * of destroyed components should be removed from DOM.
+ * of destroyed components should be disabled.
  *
  * By default, the value is set to `true`.
  * @publicApi
  */
 export const REMOVE_STYLES_ON_COMPONENT_DESTROY =
-    new InjectionToken<boolean>('RemoveStylesOnCompDestroy', {
+    new InjectionToken<boolean>(ngDevMode ? 'RemoveStylesOnCompDestroy' : '', {
       providedIn: 'root',
       factory: () => REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT,
     });
@@ -70,7 +70,7 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
       private readonly eventManager: EventManager,
       private readonly sharedStylesHost: SharedStylesHost,
       @Inject(APP_ID) private readonly appId: string,
-      @Inject(REMOVE_STYLES_ON_COMPONENT_DESTROY) private removeStylesOnCompDestroy: boolean,
+      @Inject(REMOVE_STYLES_ON_COMPONENT_DESTROY) private disableStylesOnCompDestroy: boolean,
       @Inject(DOCUMENT) private readonly doc: Document,
       @Inject(PLATFORM_ID) readonly platformId: Object,
       readonly ngZone: NgZone,
@@ -112,13 +112,13 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
       const ngZone = this.ngZone;
       const eventManager = this.eventManager;
       const sharedStylesHost = this.sharedStylesHost;
-      const removeStylesOnCompDestroy = this.removeStylesOnCompDestroy;
+      const disableStylesOnCompDestroy = this.disableStylesOnCompDestroy;
       const platformIsServer = this.platformIsServer;
 
       switch (type.encapsulation) {
         case ViewEncapsulation.Emulated:
           renderer = new EmulatedEncapsulationDomRenderer2(
-              eventManager, sharedStylesHost, type, this.appId, removeStylesOnCompDestroy, doc,
+              eventManager, sharedStylesHost, type, this.appId, disableStylesOnCompDestroy, doc,
               ngZone, platformIsServer);
           break;
         case ViewEncapsulation.ShadowDom:
@@ -127,7 +127,7 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
               platformIsServer);
         default:
           renderer = new NoneEncapsulationDomRenderer(
-              eventManager, sharedStylesHost, type, removeStylesOnCompDestroy, doc, ngZone,
+              eventManager, sharedStylesHost, type, disableStylesOnCompDestroy, doc, ngZone,
               platformIsServer);
           break;
       }
@@ -395,32 +395,28 @@ class ShadowDomRenderer extends DefaultDomRenderer2 {
 }
 
 class NoneEncapsulationDomRenderer extends DefaultDomRenderer2 {
-  private readonly styles: string[];
-
+  protected styles: string[];
   constructor(
       eventManager: EventManager,
       private readonly sharedStylesHost: SharedStylesHost,
       component: RendererType2,
-      private removeStylesOnCompDestroy: boolean,
+      private disableStylesOnCompDestroy: boolean,
       doc: Document,
       ngZone: NgZone,
       platformIsServer: boolean,
-      compId?: string,
   ) {
     super(eventManager, doc, ngZone, platformIsServer);
-    this.styles = compId ? shimStylesContent(compId, component.styles) : component.styles;
+    this.styles = component.styles;
   }
 
   applyStyles(): void {
     this.sharedStylesHost.addStyles(this.styles);
   }
 
   override destroy(): void {
-    if (!this.removeStylesOnCompDestroy) {
-      return;
+    if (this.disableStylesOnCompDestroy) {
+      this.sharedStylesHost.disableStyles(this.styles);
     }
-
-    this.sharedStylesHost.removeStyles(this.styles);
   }
 }
 
@@ -430,12 +426,14 @@ class EmulatedEncapsulationDomRenderer2 extends NoneEncapsulationDomRenderer {
 
   constructor(
       eventManager: EventManager, sharedStylesHost: SharedStylesHost, component: RendererType2,
-      appId: string, removeStylesOnCompDestroy: boolean, doc: Document, ngZone: NgZone,
+      appId: string, disableStylesOnCompDestroy: boolean, doc: Document, ngZone: NgZone,
       platformIsServer: boolean) {
-    const compId = appId + '-' + component.id;
     super(
-        eventManager, sharedStylesHost, component, removeStylesOnCompDestroy, doc, ngZone,
-        platformIsServer, compId);
+        eventManager, sharedStylesHost, component, disableStylesOnCompDestroy, doc, ngZone,
+        platformIsServer);
+
+    const compId = appId + '-' + component.id;
+    this.styles = shimStylesContent(compId, component.styles);
     this.contentAttr = shimContentAttribute(compId);
     this.hostAttr = shimHostAttribute(compId);
   }