Skip to content

Commit

Permalink
Revert "perf(platform-browser): disable styles of removed components …
Browse files Browse the repository at this point in the history
…instead of removing (#51808)" (#52238)

This reverts commit 65786b2 due to an oberved perf regression in Pantheon.

See: http://b/303667704

PR Close #52238
  • Loading branch information
alan-agius4 authored and pkozlowski-opensource committed Oct 18, 2023
1 parent b63354e commit 1640743
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 261 deletions.
Expand Up @@ -827,9 +827,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "documentElement"
},
Expand Down Expand Up @@ -1259,9 +1256,6 @@
{
"name": "ngZoneApplicationErrorHandlerFactory"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
Expand Up @@ -893,9 +893,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "documentElement"
},
Expand Down Expand Up @@ -1331,9 +1328,6 @@
{
"name": "noSideEffects"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
Expand Up @@ -677,9 +677,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -1070,9 +1067,6 @@
{
"name": "noSideEffects"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
6 changes: 0 additions & 6 deletions packages/core/test/bundling/defer/bundle.golden_symbols.json
Expand Up @@ -758,9 +758,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -2180,9 +2177,6 @@
{
"name": "ngZoneApplicationErrorHandlerFactory"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
Expand Up @@ -920,9 +920,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -1484,9 +1481,6 @@
{
"name": "noSideEffects"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
Expand Up @@ -890,9 +890,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -1448,9 +1445,6 @@
{
"name": "noSideEffects"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
Expand Up @@ -734,9 +734,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -1142,9 +1139,6 @@
{
"name": "ngZoneApplicationErrorHandlerFactory"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
6 changes: 0 additions & 6 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -1178,9 +1178,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "dispatch"
},
Expand Down Expand Up @@ -1793,9 +1790,6 @@
{
"name": "nodeChildrenAsMap"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
Expand Up @@ -605,9 +605,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -941,9 +938,6 @@
{
"name": "ngZoneApplicationErrorHandlerFactory"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
6 changes: 0 additions & 6 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -806,9 +806,6 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableStylesheet"
},
{
"name": "empty"
},
Expand Down Expand Up @@ -1289,9 +1286,6 @@
{
"name": "noSideEffects"
},
{
"name": "nonNegativeNumber"
},
{
"name": "nonNull"
},
Expand Down
36 changes: 19 additions & 17 deletions packages/platform-browser/src/dom/dom_renderer.ts
Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
Expand Down

0 comments on commit 1640743

Please sign in to comment.