Revert "perf(platform-browser): disable styles of removed components instead of removing (#51808)" (#52238)

alan-agius4 · pkozlowski-opensource · commit 1640743b18e0 · 2023-10-18T11:34:21.000+02:00
This reverts commit 65786b2 due to an oberved perf regression in Pantheon. See: http://b/303667704 PR Close #52238
diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
@@ -827,9 +827,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "documentElement"
   },
@@ -1259,9 +1256,6 @@
   {
     "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
@@ -893,9 +893,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "documentElement"
   },
@@ -1331,9 +1328,6 @@
   {
     "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
@@ -677,9 +677,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -1070,9 +1067,6 @@
   {
     "name": "noSideEffects"
   },
-  {
-    "name": "nonNegativeNumber"
-  },
   {
     "name": "nonNull"
   },
diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json
@@ -758,9 +758,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -2180,9 +2177,6 @@
   {
     "name": "ngZoneApplicationErrorHandlerFactory"
   },
-  {
-    "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
@@ -920,9 +920,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -1484,9 +1481,6 @@
   {
     "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
@@ -890,9 +890,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -1448,9 +1445,6 @@
   {
     "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
@@ -734,9 +734,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -1142,9 +1139,6 @@
   {
     "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
@@ -1178,9 +1178,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "dispatch"
   },
@@ -1793,9 +1790,6 @@
   {
     "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
@@ -605,9 +605,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -941,9 +938,6 @@
   {
     "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
@@ -806,9 +806,6 @@
   {
     "name": "diPublicInInjector"
   },
-  {
-    "name": "disableStylesheet"
-  },
   {
     "name": "empty"
   },
@@ -1289,9 +1286,6 @@
   {
     "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 disabled.
+ * of destroyed components should be removed from DOM.
  *
  * By default, the value is set to `true`.
  * @publicApi
  */
 export const REMOVE_STYLES_ON_COMPONENT_DESTROY =
-    new InjectionToken<boolean>(ngDevMode ? 'RemoveStylesOnCompDestroy' : '', {
+    new InjectionToken<boolean>('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 disableStylesOnCompDestroy: boolean,
+      @Inject(REMOVE_STYLES_ON_COMPONENT_DESTROY) private removeStylesOnCompDestroy: 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 disableStylesOnCompDestroy = this.disableStylesOnCompDestroy;
+      const removeStylesOnCompDestroy = this.removeStylesOnCompDestroy;
       const platformIsServer = this.platformIsServer;
 
       switch (type.encapsulation) {
         case ViewEncapsulation.Emulated:
           renderer = new EmulatedEncapsulationDomRenderer2(
-              eventManager, sharedStylesHost, type, this.appId, disableStylesOnCompDestroy, doc,
+              eventManager, sharedStylesHost, type, this.appId, removeStylesOnCompDestroy, 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, disableStylesOnCompDestroy, doc, ngZone,
+              eventManager, sharedStylesHost, type, removeStylesOnCompDestroy, doc, ngZone,
               platformIsServer);
           break;
       }
@@ -403,28 +403,32 @@ class ShadowDomRenderer extends DefaultDomRenderer2 {
 }
 
 class NoneEncapsulationDomRenderer extends DefaultDomRenderer2 {
-  protected styles: string[];
+  private readonly styles: string[];
+
   constructor(
       eventManager: EventManager,
       private readonly sharedStylesHost: SharedStylesHost,
       component: RendererType2,
-      private disableStylesOnCompDestroy: boolean,
+      private removeStylesOnCompDestroy: boolean,
       doc: Document,
       ngZone: NgZone,
       platformIsServer: boolean,
+      compId?: string,
   ) {
     super(eventManager, doc, ngZone, platformIsServer);
-    this.styles = component.styles;
+    this.styles = compId ? shimStylesContent(compId, component.styles) : component.styles;
   }
 
   applyStyles(): void {
     this.sharedStylesHost.addStyles(this.styles);
   }
 
   override destroy(): void {
-    if (this.disableStylesOnCompDestroy) {
-      this.sharedStylesHost.disableStyles(this.styles);
+    if (!this.removeStylesOnCompDestroy) {
+      return;
     }
+
+    this.sharedStylesHost.removeStyles(this.styles);
   }
 }
 
@@ -434,14 +438,12 @@ class EmulatedEncapsulationDomRenderer2 extends NoneEncapsulationDomRenderer {
 
   constructor(
       eventManager: EventManager, sharedStylesHost: SharedStylesHost, component: RendererType2,
-      appId: string, disableStylesOnCompDestroy: boolean, doc: Document, ngZone: NgZone,
+      appId: string, removeStylesOnCompDestroy: boolean, doc: Document, ngZone: NgZone,
       platformIsServer: boolean) {
-    super(
-        eventManager, sharedStylesHost, component, disableStylesOnCompDestroy, doc, ngZone,
-        platformIsServer);
-
     const compId = appId + '-' + component.id;
-    this.styles = shimStylesContent(compId, component.styles);
+    super(
+        eventManager, sharedStylesHost, component, removeStylesOnCompDestroy, doc, ngZone,
+        platformIsServer, compId);
     this.contentAttr = shimContentAttribute(compId);
     this.hostAttr = shimHostAttribute(compId);
   }
diff --git a/packages/platform-browser/src/dom/shared_styles_host.ts b/packages/platform-browser/src/dom/shared_styles_host.ts
diff --git a/packages/platform-browser/test/dom/dom_renderer_spec.ts b/packages/platform-browser/test/dom/dom_renderer_spec.ts
diff --git a/packages/platform-browser/test/dom/shared_styles_host_spec.ts b/packages/platform-browser/test/dom/shared_styles_host_spec.ts

Original file line number	Diff line number	Diff line change
`@@ -827,9 +827,6 @@`
`827`	`827`	`{`
`828`	`828`	`"name": "diPublicInInjector"`
`829`	`829`	`},`
`830`		`- {`
`831`		`- "name": "disableStylesheet"`
`832`		`- },`
`833`	`830`	`{`
`834`	`831`	`"name": "documentElement"`
`835`	`832`	`},`
`@@ -1259,9 +1256,6 @@`
`1259`	`1256`	`{`
`1260`	`1257`	`"name": "ngZoneApplicationErrorHandlerFactory"`
`1261`	`1258`	`},`
`1262`		`- {`
`1263`		`- "name": "nonNegativeNumber"`
`1264`		`- },`
`1265`	`1259`	`{`
`1266`	`1260`	`"name": "nonNull"`
`1267`	`1261`	`},`
Original file line number	Diff line number	Diff line change
`@@ -893,9 +893,6 @@`
`893`	`893`	`{`
`894`	`894`	`"name": "diPublicInInjector"`
`895`	`895`	`},`
`896`		`- {`
`897`		`- "name": "disableStylesheet"`
`898`		`- },`
`899`	`896`	`{`
`900`	`897`	`"name": "documentElement"`
`901`	`898`	`},`
`@@ -1331,9 +1328,6 @@`
`1331`	`1328`	`{`
`1332`	`1329`	`"name": "noSideEffects"`
`1333`	`1330`	`},`
`1334`		`- {`
`1335`		`- "name": "nonNegativeNumber"`
`1336`		`- },`
`1337`	`1331`	`{`
`1338`	`1332`	`"name": "nonNull"`
`1339`	`1333`	`},`
Original file line number	Diff line number	Diff line change
`@@ -677,9 +677,6 @@`
`677`	`677`	`{`
`678`	`678`	`"name": "diPublicInInjector"`
`679`	`679`	`},`
`680`		`- {`
`681`		`- "name": "disableStylesheet"`
`682`		`- },`
`683`	`680`	`{`
`684`	`681`	`"name": "empty"`
`685`	`682`	`},`
`@@ -1070,9 +1067,6 @@`
`1070`	`1067`	`{`
`1071`	`1068`	`"name": "noSideEffects"`
`1072`	`1069`	`},`
`1073`		`- {`
`1074`		`- "name": "nonNegativeNumber"`
`1075`		`- },`
`1076`	`1070`	`{`
`1077`	`1071`	`"name": "nonNull"`
`1078`	`1072`	`},`
Original file line number	Diff line number	Diff line change
`@@ -758,9 +758,6 @@`
`758`	`758`	`{`
`759`	`759`	`"name": "diPublicInInjector"`
`760`	`760`	`},`
`761`		`- {`
`762`		`- "name": "disableStylesheet"`
`763`		`- },`
`764`	`761`	`{`
`765`	`762`	`"name": "empty"`
`766`	`763`	`},`
`@@ -2180,9 +2177,6 @@`
`2180`	`2177`	`{`
`2181`	`2178`	`"name": "ngZoneApplicationErrorHandlerFactory"`
`2182`	`2179`	`},`
`2183`		`- {`
`2184`		`- "name": "nonNegativeNumber"`
`2185`		`- },`
`2186`	`2180`	`{`
`2187`	`2181`	`"name": "nonNull"`
`2188`	`2182`	`},`
Original file line number	Diff line number	Diff line change
`@@ -920,9 +920,6 @@`
`920`	`920`	`{`
`921`	`921`	`"name": "diPublicInInjector"`
`922`	`922`	`},`
`923`		`- {`
`924`		`- "name": "disableStylesheet"`
`925`		`- },`
`926`	`923`	`{`
`927`	`924`	`"name": "empty"`
`928`	`925`	`},`
`@@ -1484,9 +1481,6 @@`
`1484`	`1481`	`{`
`1485`	`1482`	`"name": "noSideEffects"`
`1486`	`1483`	`},`
`1487`		`- {`
`1488`		`- "name": "nonNegativeNumber"`
`1489`		`- },`
`1490`	`1484`	`{`
`1491`	`1485`	`"name": "nonNull"`
`1492`	`1486`	`},`
Original file line number	Diff line number	Diff line change
`@@ -890,9 +890,6 @@`
`890`	`890`	`{`
`891`	`891`	`"name": "diPublicInInjector"`
`892`	`892`	`},`
`893`		`- {`
`894`		`- "name": "disableStylesheet"`
`895`		`- },`
`896`	`893`	`{`
`897`	`894`	`"name": "empty"`
`898`	`895`	`},`
`@@ -1448,9 +1445,6 @@`
`1448`	`1445`	`{`
`1449`	`1446`	`"name": "noSideEffects"`
`1450`	`1447`	`},`
`1451`		`- {`
`1452`		`- "name": "nonNegativeNumber"`
`1453`		`- },`
`1454`	`1448`	`{`
`1455`	`1449`	`"name": "nonNull"`
`1456`	`1450`	`},`
Original file line number	Diff line number	Diff line change
`@@ -734,9 +734,6 @@`
`734`	`734`	`{`
`735`	`735`	`"name": "diPublicInInjector"`
`736`	`736`	`},`
`737`		`- {`
`738`		`- "name": "disableStylesheet"`
`739`		`- },`
`740`	`737`	`{`
`741`	`738`	`"name": "empty"`
`742`	`739`	`},`
`@@ -1142,9 +1139,6 @@`
`1142`	`1139`	`{`
`1143`	`1140`	`"name": "ngZoneApplicationErrorHandlerFactory"`
`1144`	`1141`	`},`
`1145`		`- {`
`1146`		`- "name": "nonNegativeNumber"`
`1147`		`- },`
`1148`	`1142`	`{`
`1149`	`1143`	`"name": "nonNull"`
`1150`	`1144`	`},`
Original file line number	Diff line number	Diff line change
`@@ -1178,9 +1178,6 @@`
`1178`	`1178`	`{`
`1179`	`1179`	`"name": "diPublicInInjector"`
`1180`	`1180`	`},`
`1181`		`- {`
`1182`		`- "name": "disableStylesheet"`
`1183`		`- },`
`1184`	`1181`	`{`
`1185`	`1182`	`"name": "dispatch"`
`1186`	`1183`	`},`
`@@ -1793,9 +1790,6 @@`
`1793`	`1790`	`{`
`1794`	`1791`	`"name": "nodeChildrenAsMap"`
`1795`	`1792`	`},`
`1796`		`- {`
`1797`		`- "name": "nonNegativeNumber"`
`1798`		`- },`
`1799`	`1793`	`{`
`1800`	`1794`	`"name": "nonNull"`
`1801`	`1795`	`},`
Original file line number	Diff line number	Diff line change
`@@ -605,9 +605,6 @@`
`605`	`605`	`{`
`606`	`606`	`"name": "diPublicInInjector"`
`607`	`607`	`},`
`608`		`- {`
`609`		`- "name": "disableStylesheet"`
`610`		`- },`
`611`	`608`	`{`
`612`	`609`	`"name": "empty"`
`613`	`610`	`},`
`@@ -941,9 +938,6 @@`
`941`	`938`	`{`
`942`	`939`	`"name": "ngZoneApplicationErrorHandlerFactory"`
`943`	`940`	`},`
`944`		`- {`
`945`		`- "name": "nonNegativeNumber"`
`946`		`- },`
`947`	`941`	`{`
`948`	`942`	`"name": "nonNull"`
`949`	`943`	`},`
Original file line number	Diff line number	Diff line change
`@@ -806,9 +806,6 @@`
`806`	`806`	`{`
`807`	`807`	`"name": "diPublicInInjector"`
`808`	`808`	`},`
`809`		`- {`
`810`		`- "name": "disableStylesheet"`
`811`		`- },`
`812`	`809`	`{`
`813`	`810`	`"name": "empty"`
`814`	`811`	`},`
`@@ -1289,9 +1286,6 @@`
`1289`	`1286`	`{`
`1290`	`1287`	`"name": "noSideEffects"`
`1291`	`1288`	`},`
`1292`		`- {`
`1293`		`- "name": "nonNegativeNumber"`
`1294`		`- },`
`1295`	`1289`	`{`
`1296`	`1290`	`"name": "nonNull"`
`1297`	`1291`	`},`