Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/core/src/render3/interfaces/shared_styles_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,19 @@ export interface SharedStylesHost {
*/
addStyles(styles: string[], urls?: string[]): void;

/**
* Disables styles of destroyed components instead of removing them from the DOM.
* This avoids expensive style recalculations triggered by DOM removal.
* @param styles An array of style content strings.
* @param urls An array of URLs to be disabled as link tags.
*/
disableStyles(styles: string[], urls?: string[]): void;

/**
* Removes embedded styles from the DOM that were added as HTML `style` elements.
* @param styles An array of style content strings.
* @param urls An array of URLs to be removed as link tags.
* @deprecated Use `disableStyles` instead.
*/
removeStyles(styles: string[], urls?: string[]): void;

Expand Down
47 changes: 47 additions & 0 deletions packages/core/test/acceptance/hmr_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {DOCUMENT} from '@angular/common';
import {computeMsgId} from '@angular/compiler';
import {TestBed} from '@angular/core/testing';
import {clearTranslations, loadTranslations} from '@angular/localize';
Expand Down Expand Up @@ -333,6 +334,52 @@ describe('hot module replacement', () => {
);
});

it('should clean up stale disabled stylesheets when styles change during HMR', () => {
const initialStyles = `strong { color: red; }`;
const initialMetadata: Component = {
encapsulation: ViewEncapsulation.None,
selector: 'child-cmp',
template: 'Hello <strong>{{state}}</strong>',
styles: initialStyles,
};

@Component(initialMetadata)
class ChildCmp {
state = 0;
}

@Component({
imports: [ChildCmp],
template: `<child-cmp />`,
})
class RootCmp {}

const fixture = TestBed.createComponent(RootCmp);
fixture.detectChanges();

const doc = TestBed.inject(DOCUMENT);
const findStyle = (text: string) =>
Array.from(doc.head!.querySelectorAll('style')).find((s) => s.textContent === text);

// Verify initial style is present
expect(findStyle(initialStyles)).toBeTruthy();

const newStyles = `strong { background: pink; }`;

// Simulate HMR with different styles
replaceMetadata(ChildCmp, {
...initialMetadata,
template: `Changed <strong>{{state}}</strong>!`,
styles: newStyles,
});
fixture.detectChanges();

// Old style should be fully removed from the DOM, not just disabled
expect(findStyle(initialStyles)).toBeFalsy();
// New style should be present
expect(findStyle(newStyles)).toBeTruthy();
});

it('should continue binding inputs to a component that is replaced', () => {
const initialMetadata: Component = {
selector: 'child-cmp',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,13 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentElement",
"documentSupported",
"domOnlyFirstCreatePass",
"elementLikeEndShared",
"elementLikeStartShared",
"enableStyles",
"enterDI",
"enterView",
"epoch",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,9 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentSupported",
"enableStyles",
"enterDI",
"enterView",
"epoch",
Expand Down
2 changes: 2 additions & 0 deletions packages/core/test/bundling/defer/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
"createLinkElement",
"createProvidersConfig",
"createStyleElement",
"disableStyles",
"enableStyles",
"errorHandler",
"getBaseElementHref",
"getDOM",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,12 +521,14 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentSupported",
"effect",
"elementAttributeInternal",
"elementLikeEndShared",
"elementLikeStartShared",
"emailValidator",
"enableStyles",
"enterDI",
"enterView",
"epoch",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,12 +518,14 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentSupported",
"effect",
"elementAttributeInternal",
"elementLikeEndShared",
"elementLikeStartShared",
"emailValidator",
"enableStyles",
"enterDI",
"enterView",
"epoch",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentSupported",
"enableApplyRootElementTransformImpl",
"enableFindMatchingDehydratedViewImpl",
Expand All @@ -513,6 +514,7 @@
"enableRetrieveDeferBlockDataImpl",
"enableRetrieveHydrationInfoImpl",
"enableStashEventListenerImpl",
"enableStyles",
"enterDI",
"enterSkipHydrationBlock",
"enterView",
Expand Down
2 changes: 2 additions & 0 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -577,12 +577,14 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentSupported",
"domOnlyFirstCreatePass",
"elementAttributeInternal",
"elementLikeEndShared",
"elementLikeStartShared",
"emptyPathMatch",
"enableStyles",
"encodeUriFragment",
"encodeUriQuery",
"encodeUriSegment",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,9 @@
"diPublicInInjector",
"directiveHostEndFirstCreatePass",
"directiveHostFirstCreatePass",
"disableStyles",
"documentSupported",
"enableStyles",
"enterDI",
"enterView",
"epoch",
Expand Down
43 changes: 30 additions & 13 deletions packages/platform-browser/src/dom/dom_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT = true;

/**
* A DI token 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
Expand Down Expand Up @@ -140,7 +140,7 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
private readonly eventManager: EventManager,
@Inject(SHARED_STYLES_HOST) 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,
readonly ngZone: NgZone,
@Inject(CSP_NONCE) private readonly nonce: string | null = null,
Expand Down Expand Up @@ -187,7 +187,7 @@ 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 tracingService = this.tracingService;

switch (type.encapsulation) {
Expand All @@ -197,7 +197,7 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
sharedStylesHost,
type,
this.appId,
removeStylesOnCompDestroy,
disableStylesOnCompDestroy,
doc,
ngZone,
tracingService,
Expand Down Expand Up @@ -230,7 +230,7 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
eventManager,
sharedStylesHost,
type,
removeStylesOnCompDestroy,
disableStylesOnCompDestroy,
doc,
ngZone,
tracingService,
Expand All @@ -253,6 +253,10 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
* @param componentId ID of the component that is being replaced.
*/
protected componentReplaced(componentId: string) {
const renderer = this.rendererByCompId.get(componentId);
if (renderer instanceof NoneEncapsulationDomRenderer) {
renderer.removeStyles();
}
this.rendererByCompId.delete(componentId);
}
}
Expand Down Expand Up @@ -578,14 +582,14 @@ class ShadowDomRenderer extends DefaultDomRenderer2 {
}

class NoneEncapsulationDomRenderer extends DefaultDomRenderer2 {
private readonly styles: string[];
private readonly styleUrls?: string[];
protected styles: string[];
private styleUrls?: string[];

constructor(
eventManager: EventManager,
private readonly sharedStylesHost: SharedStylesHost,
component: RendererType2,
private removeStylesOnCompDestroy: boolean,
private disableStylesOnCompDestroy: boolean,
doc: Document,
ngZone: NgZone,
tracingService: TracingService<TracingSnapshot> | null,
Expand All @@ -599,20 +603,32 @@ class NoneEncapsulationDomRenderer extends DefaultDomRenderer2 {
styles = addBaseHrefToCssSourceMap(baseHref, styles);
}

this.styles = compId ? shimStylesContent(compId, styles) : styles;
this.styles = styles;
this.styleUrls = component.getExternalStyles?.(compId);
}

applyStyles(): void {
this.sharedStylesHost.addStyles(this.styles, this.styleUrls);
}

removeStyles(): void {
this.sharedStylesHost.removeUsagesAndElements(this.styles, this.styleUrls);
// Clear the styles so that any future `destroy()` call on this renderer is a
// no-op. This is necessary because `BaseAnimationRenderer.destroy()` defers
// `delegate.destroy()` via `afterFlushAnimationsDone` + `queueMicrotask`.
// Without this, the deferred `disableStyles()` would find the *replacement*
// component's newly-added style record (same content key) and disable it.
Comment thread
mattlewis92 marked this conversation as resolved.
// TODO: clean this up when the legacy animation package (`@angular/platform-browser/animations`) is removed.
this.styles = [];
this.styleUrls = undefined;
}

override destroy(): void {
if (!this.removeStylesOnCompDestroy) {
if (!this.disableStylesOnCompDestroy) {
return;
}
if (allLeavingAnimations.size === 0) {
this.sharedStylesHost.removeStyles(this.styles, this.styleUrls);
this.sharedStylesHost.disableStyles(this.styles, this.styleUrls);
}
}
}
Expand All @@ -626,7 +642,7 @@ class EmulatedEncapsulationDomRenderer2 extends NoneEncapsulationDomRenderer {
sharedStylesHost: SharedStylesHost,
component: RendererType2,
appId: string,
removeStylesOnCompDestroy: boolean,
disableStylesOnCompDestroy: boolean,
doc: Document,
ngZone: NgZone,
tracingService: TracingService<TracingSnapshot> | null,
Expand All @@ -636,12 +652,13 @@ class EmulatedEncapsulationDomRenderer2 extends NoneEncapsulationDomRenderer {
eventManager,
sharedStylesHost,
component,
removeStylesOnCompDestroy,
disableStylesOnCompDestroy,
doc,
ngZone,
tracingService,
compId,
);
this.styles = shimStylesContent(compId, component.styles);
this.contentAttr = shimContentAttribute(compId);
this.hostAttr = shimHostAttribute(compId);
}
Expand Down
Loading
Loading