Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
refactor(core): track multiple usages of a single host in `SharedStyl…
…esHost`

When rendering components underneath shadow roots, it is possible to render multiple times, meaning `SharedStylesHost.prototype.addHost` will potentially be called multiple times on the same shadow root. This is reasonable, and we need to make sure that as long as any one component is still using a given shadow root as its host, we should continue applying styles there. Only when all usages of the host are removed do we stop adding styles to it.
  • Loading branch information
dgp1130 committed Mar 28, 2026
commit 38602a98218e9944fa5dbcfd8ae4de1bd891aee6
40 changes: 27 additions & 13 deletions packages/platform-browser/src/dom/shared_styles_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
private readonly external = new Map<string /** URL */, UsageRecord<HTMLLinkElement>>();

/**
* Set of host DOM nodes that will have styles attached.
* The set of DOM nodes with styles attached, tracked by usage count.
*/
private readonly hosts = new Set<Node>();
private readonly hosts = new Map<Node, number>();

constructor(
@Inject(DOCUMENT) private readonly doc: Document,
Expand All @@ -130,7 +130,7 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
@Inject(PLATFORM_ID) platformId: object = {},
) {
addServerStyles(doc, appId, this.inline, this.external);
this.hosts.add(doc.head);
this.hosts.set(doc.head, 1);
}

addStyles(styles: string[], urls?: string[]): void {
Expand Down Expand Up @@ -173,7 +173,9 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
// Otherwise, create an entry to track the elements and add element for each host
usages.set(value, {
usage: 1,
elements: [...this.hosts].map((host) => this.addElement(host, creator(value, this.doc))),
elements: [...this.hosts.keys()].map((host) =>
this.addElement(host, creator(value, this.doc)),
),
});
}
}
Expand Down Expand Up @@ -204,19 +206,31 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
}

addHost(hostNode: Node): void {
this.hosts.add(hostNode);

// Add existing styles to new host
for (const [style, {elements}] of this.inline) {
elements.push(this.addElement(hostNode, createStyleElement(style, this.doc)));
}
for (const [url, {elements}] of this.external) {
elements.push(this.addElement(hostNode, createLinkElement(url, this.doc)));
const existingUsage = this.hosts.get(hostNode) ?? 0;
if (existingUsage === 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: Can we move the this.hosts.set(hostNode, existingUsage + 1); up and invert this guard to reduce nesting?

// Add existing styles to new host
for (const [style, {elements}] of this.inline) {
elements.push(this.addElement(hostNode, createStyleElement(style, this.doc)));
}
for (const [url, {elements}] of this.external) {
elements.push(this.addElement(hostNode, createLinkElement(url, this.doc)));
}
}

this.hosts.set(hostNode, existingUsage + 1);
}

removeHost(hostNode: Node): void {
this.hosts.delete(hostNode);
const usage = this.hosts.get(hostNode);
if (typeof ngDevMode !== 'undefined' && ngDevMode && usage === undefined) {
Comment thread
dgp1130 marked this conversation as resolved.
throw new Error('Attempted to remove a host which was not added.');
}

if (usage! === 1) {
this.hosts.delete(hostNode);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also be removing the inline and external styles when the last host is removed?

} else {
this.hosts.set(hostNode, usage! - 1);
}
}

private addElement<T extends HTMLElement>(host: Node, element: T): T {
Expand Down
40 changes: 40 additions & 0 deletions packages/platform-browser/test/dom/shared_styles_host_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,26 @@ describe('SharedStylesHost', () => {
expect(someHost.innerHTML).toEqual('<style>a {};</style>');
});

it('should track the same host added multiple times', () => {
// Add same host twice.
ssh.addHost(someHost);
ssh.addHost(someHost);

// Styles are added to host.
ssh.addStyles(['a {};']);
expect(someHost.innerHTML).toContain('<style>a {};</style>');

// Only remove host once, styles should continue to be added.
ssh.removeHost(someHost);
ssh.addStyles(['b {};']);
expect(someHost.innerHTML).toContain('<style>b {};</style>');

// Remove host second time, new styles are no longer added.
ssh.removeHost(someHost);
ssh.addStyles(['c {};']);
expect(someHost.innerHTML).not.toContain('<style>c {};</style>');
});

it('should add styles only once to hosts', () => {
ssh.addStyles(['a {};']);
ssh.addHost(someHost);
Expand Down Expand Up @@ -88,6 +108,26 @@ describe('SharedStylesHost', () => {
expect(someHost.innerHTML).toEqual('<link rel="stylesheet" href="component-1.css">');
});

it('should track the same host added multiple times', () => {
// Add same host twice.
ssh.addHost(someHost);
ssh.addHost(someHost);

// Styles are added to host.
ssh.addStyles([], ['component-1.css']);
expect(someHost.innerHTML).toContain('<link rel="stylesheet" href="component-1.css">');

// Only remove host once, styles should continue to be added.
ssh.removeHost(someHost);
ssh.addStyles([], ['component-2.css']);
expect(someHost.innerHTML).toContain('<link rel="stylesheet" href="component-2.css">');

// Remove host second time, new styles are no longer added.
ssh.removeHost(someHost);
ssh.addStyles([], ['component-3.css']);
expect(someHost.innerHTML).not.toContain('<link rel="stylesheet" href="component-3.css">');
});

it('should add styles only once to hosts', () => {
ssh.addStyles([], ['component-1.css']);
ssh.addHost(someHost);
Expand Down