From 52be35118feee587d2efe5a6c55502c171caaa97 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 3 Dec 2024 16:30:29 -0500 Subject: [PATCH] fix(platform-browser): collect external component styles from server rendering (#59031) SSR generated component styles used in development environments will add external styles via link elements to the HTML. However, the runtime would previously not collect these link elements for reuse with rendered components. This would result in two copies of the link elements present in the DOM. In isolation this is not problematic as it is only present in development mode. Unfortunately, the Vite-based CSS HMR functionality used by the Angular CLI only updates the first stylesheet it finds and leaves other instances of the stylesheet in place. This behavior causes the styles to be left in an inconsistent state. This could be considered a defect within Vite as it should update all relevant styles to maintain consistency but ideally there should not be two instances in the Angular SSR case. To avoid the Vite issue, the runtime will now collect SSR generated external styles and reuse them. PR Close #59031 --- .../src/dom/shared_styles_host.ts | 29 ++++++++++++------- .../test/dom/shared_styles_host_spec.ts | 27 +++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/packages/platform-browser/src/dom/shared_styles_host.ts b/packages/platform-browser/src/dom/shared_styles_host.ts index b34ed4c0da4..d9829e1f945 100644 --- a/packages/platform-browser/src/dom/shared_styles_host.ts +++ b/packages/platform-browser/src/dom/shared_styles_host.ts @@ -57,22 +57,31 @@ function createStyleElement(style: string, doc: Document): HTMLStyleElement { * identifier attribute (`ng-app-id`) to the provide identifier and adds usage records for each. * @param doc An HTML DOM document instance. * @param appId A string containing an Angular application identifer. - * @param usages A Map object for tracking style usage. + * @param inline A Map object for tracking inline (defined via `styles` in component decorator) style usage. + * @param external A Map object for tracking external (defined via `styleUrls` in component decorator) style usage. */ function addServerStyles( doc: Document, appId: string, - usages: Map>, + inline: Map>, + external: Map>, ): void { - const styleElements = doc.head?.querySelectorAll( - `style[${APP_ID_ATTRIBUTE_NAME}="${appId}"]`, + const elements = doc.head?.querySelectorAll( + `style[${APP_ID_ATTRIBUTE_NAME}="${appId}"],link[${APP_ID_ATTRIBUTE_NAME}="${appId}"]`, ); - if (styleElements) { - for (const styleElement of styleElements) { - if (styleElement.textContent) { - styleElement.removeAttribute(APP_ID_ATTRIBUTE_NAME); - usages.set(styleElement.textContent, {usage: 0, elements: [styleElement]}); + if (elements) { + for (const styleElement of elements) { + styleElement.removeAttribute(APP_ID_ATTRIBUTE_NAME); + if (styleElement instanceof HTMLLinkElement) { + // Only use filename from href + // The href is build time generated with a unique value to prevent duplicates. + external.set(styleElement.href.slice(styleElement.href.lastIndexOf('/') + 1), { + usage: 0, + elements: [styleElement], + }); + } else if (styleElement.textContent) { + inline.set(styleElement.textContent, {usage: 0, elements: [styleElement]}); } } } @@ -123,7 +132,7 @@ export class SharedStylesHost implements OnDestroy { @Inject(PLATFORM_ID) platformId: object = {}, ) { this.isServer = isPlatformServer(platformId); - addServerStyles(doc, appId, this.inline); + addServerStyles(doc, appId, this.inline, this.external); this.hosts.add(doc.head); } diff --git a/packages/platform-browser/test/dom/shared_styles_host_spec.ts b/packages/platform-browser/test/dom/shared_styles_host_spec.ts index 38ffd2765ef..04a19937a00 100644 --- a/packages/platform-browser/test/dom/shared_styles_host_spec.ts +++ b/packages/platform-browser/test/dom/shared_styles_host_spec.ts @@ -61,6 +61,18 @@ describe('SharedStylesHost', () => { ssh.addHost(someHost); expect(someHost.innerHTML).toEqual(''); }); + + it(`should reuse SSR generated element`, () => { + const style = doc.createElement('style'); + style.setAttribute('ng-app-id', 'app-id'); + style.textContent = 'a {};'; + doc.head.appendChild(style); + + ssh = new SharedStylesHost(doc, 'app-id'); + ssh.addStyles(['a {};']); + expect(doc.head.innerHTML).toContain(''); + expect(doc.head.innerHTML).not.toContain('ng-app-id'); + }); }); describe('external', () => { @@ -114,5 +126,20 @@ describe('SharedStylesHost', () => { '', ); }); + + it(`should reuse SSR generated element`, () => { + const link = doc.createElement('link'); + link.setAttribute('rel', 'stylesheet'); + link.setAttribute('href', 'component-1.css'); + link.setAttribute('ng-app-id', 'app-id'); + doc.head.appendChild(link); + + ssh = new SharedStylesHost(doc, 'app-id'); + ssh.addStyles([], ['component-1.css']); + expect(doc.head.innerHTML).toContain( + '', + ); + expect(doc.head.innerHTML).not.toContain('ng-app-id'); + }); }); });