Skip to content

Commit

Permalink
fix(platform-browser): only add ng-app-id to style on server side (#…
Browse files Browse the repository at this point in the history
…49465)

This commit fixes an issue where it causes issues in G3 when we add `ng-app-id` on browser apps.

PR Close #49465
  • Loading branch information
alan-agius4 authored and pkozlowski-opensource committed Mar 17, 2023
1 parent 20f7187 commit c934a8e
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 12 deletions.
6 changes: 3 additions & 3 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -2,7 +2,7 @@
"cli-hello-world": {
"uncompressed": {
"runtime": 908,
"main": 128561,
"main": 129184,
"polyfills": 33792
}
},
Expand Down Expand Up @@ -41,14 +41,14 @@
"animations": {
"uncompressed": {
"runtime": 898,
"main": 159415,
"main": 159979,
"polyfills": 33782
}
},
"standalone-bootstrap": {
"uncompressed": {
"runtime": 918,
"main": 86975,
"main": 87601,
"polyfills": 33802
}
},
Expand Down
2 changes: 1 addition & 1 deletion integration/platform-server/e2e/helloworld-spec.ts
Expand Up @@ -28,7 +28,7 @@ describe('Hello world E2E Tests', function() {
expect(element(by.css('div')).getText()).toEqual('Hello world!');

// Make sure the server styles get reused by the client.
expect(element(by.css('style[ng-app-id="ng"]')).isPresent()).toBeTruthy();
expect(element(by.css('style[ng-app-id="ng"]')).isPresent()).toBeFalsy();
expect(element(by.css('style[ng-style-reused]')).isPresent()).toBeTruthy();
expect(element(by.css('style')).getText()).toBe('');

Expand Down
17 changes: 13 additions & 4 deletions packages/platform-browser/src/dom/shared_styles_host.ts
Expand Up @@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DOCUMENT} from '@angular/common';
import {APP_ID, CSP_NONCE, Inject, Injectable, OnDestroy, Optional} from '@angular/core';
import {DOCUMENT, isPlatformServer} from '@angular/common';
import {APP_ID, CSP_NONCE, Inject, Injectable, OnDestroy, Optional, PLATFORM_ID} from '@angular/core';

/** The style elements attribute name used to set value of `APP_ID` token. */
const APP_ID_ATTRIBUTE_NAME = 'ng-app-id';
Expand All @@ -22,12 +22,15 @@ export class SharedStylesHost implements OnDestroy {
> ();
private readonly hostNodes = new Set<Node>();
private readonly styleNodesInDOM: Map<string, HTMLStyleElement>|null;
private readonly platformIsServer: boolean;

constructor(
@Inject(DOCUMENT) private readonly doc: Document,
@Inject(APP_ID) private readonly appId: string,
@Inject(CSP_NONCE) @Optional() private nonce?: string|null) {
@Inject(CSP_NONCE) @Optional() private nonce?: string|null,
@Inject(PLATFORM_ID) readonly platformId: object = {}) {
this.styleNodesInDOM = this.collectServerRenderedStyles();
this.platformIsServer = isPlatformServer(platformId);
this.resetHostNodes();
}

Expand Down Expand Up @@ -132,6 +135,8 @@ export class SharedStylesHost implements OnDestroy {
// `styleNodesInDOM` cannot be undefined due to the above `styleNodesInDOM?.get`.
styleNodesInDOM!.delete(style);

styleEl.removeAttribute(APP_ID_ATTRIBUTE_NAME);

if (typeof ngDevMode === 'undefined' || ngDevMode) {
// This attribute is solely used for debugging purposes.
styleEl.setAttribute('ng-style-reused', '');
Expand All @@ -147,7 +152,11 @@ export class SharedStylesHost implements OnDestroy {
}

styleEl.textContent = style;
styleEl.setAttribute(APP_ID_ATTRIBUTE_NAME, this.appId);

if (this.platformIsServer) {
styleEl.setAttribute(APP_ID_ATTRIBUTE_NAME, this.appId);
}

return styleEl;
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/platform-browser/test/dom/shared_styles_host_spec.ts
Expand Up @@ -25,20 +25,20 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
it('should add existing styles to new hosts', () => {
ssh.addStyles(['a {};']);
ssh.addHost(someHost);
expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
expect(someHost.innerHTML).toEqual('<style>a {};</style>');
});

it('should add new styles to hosts', () => {
ssh.addHost(someHost);
ssh.addStyles(['a {};']);
expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
expect(someHost.innerHTML).toEqual('<style>a {};</style>');
});

it('should add styles only once to hosts', () => {
ssh.addStyles(['a {};']);
ssh.addHost(someHost);
ssh.addStyles(['a {};']);
expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
expect(someHost.innerHTML).toEqual('<style>a {};</style>');
});

it('should use the document head as default host', () => {
Expand All @@ -49,7 +49,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
it('should remove style nodes on destroy', () => {
ssh.addStyles(['a {};']);
ssh.addHost(someHost);
expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
expect(someHost.innerHTML).toEqual('<style>a {};</style>');

ssh.ngOnDestroy();
expect(someHost.innerHTML).toEqual('');
Expand Down

0 comments on commit c934a8e

Please sign in to comment.