refactor(platform-browser): rename ng-app style attribute to ng-app-id (#49424)

alan-agius4 · pkozlowski-opensource · commit 9636910c0158 · 2023-03-17T10:46:03.000+01:00
`ng-app` is an AngularJS attribute, see https://docs.angularjs.org/api/ng/directive/ngApp. Using this attribute on a non AngularJS element can cause DI issues in AngularJS when running an AngularJS and Angular application on the same page. As such, we avoid such problems the Angular `ng-app` attribute is renamed to `ng-app-id`. PR Close #49424
diff --git a/integration/platform-server/e2e/helloworld-spec.ts b/integration/platform-server/e2e/helloworld-spec.ts
@@ -15,7 +15,7 @@ describe('Hello world E2E Tests', function() {
     // Load the page without waiting for Angular since it is not bootstrapped automatically.
     browser.driver.get(browser.baseUrl + 'helloworld');
 
-    const style = browser.driver.findElement(by.css('style[ng-app="ng"]'));
+    const style = browser.driver.findElement(by.css('style[ng-app-id="ng"]'));
     expect(style.getText()).not.toBeNull();
 
     // Test the contents from the server.
@@ -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="ng"]')).isPresent()).toBeTruthy();
+    expect(element(by.css('style[ng-app-id="ng"]')).isPresent()).toBeTruthy();
     expect(element(by.css('style[ng-style-reused]')).isPresent()).toBeTruthy();
     expect(element(by.css('style')).getText()).toBe('');
 
diff --git a/packages/platform-browser/src/dom/shared_styles_host.ts b/packages/platform-browser/src/dom/shared_styles_host.ts
@@ -6,8 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {DOCUMENT, isPlatformServer} from '@angular/common';
-import {APP_ID, Inject, Injectable, OnDestroy, PLATFORM_ID} from '@angular/core';
+import {DOCUMENT} from '@angular/common';
+import {APP_ID, Inject, Injectable, OnDestroy} from '@angular/core';
+
+/** The style elements attribute name used to set value of `APP_ID` token. */
+const APP_ID_ATTRIBUTE_NAME = 'ng-app-id';
 
 @Injectable()
 export class SharedStylesHost implements OnDestroy {
@@ -19,14 +22,12 @@ 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 appId: string,
-      @Inject(PLATFORM_ID) readonly platformId: object = {}) {
+      @Inject(DOCUMENT) private readonly doc: Document,
+      @Inject(APP_ID) private readonly appId: string) {
     this.styleNodesInDOM = this.collectServerRenderedStyles();
     this.resetHostNodes();
-    this.platformIsServer = isPlatformServer(platformId);
   }
 
   addStyles(styles: string[]): void {
@@ -92,8 +93,8 @@ export class SharedStylesHost implements OnDestroy {
   }
 
   private collectServerRenderedStyles(): Map<string, HTMLStyleElement>|null {
-    const styles =
-        this.doc.head?.querySelectorAll<HTMLStyleElement>(`style[ng-app="${this.appId}"]`);
+    const styles = this.doc.head?.querySelectorAll<HTMLStyleElement>(
+        `style[${APP_ID_ATTRIBUTE_NAME}="${this.appId}"]`);
 
     if (styles?.length) {
       const styleMap = new Map<string, HTMLStyleElement>();
@@ -139,12 +140,7 @@ export class SharedStylesHost implements OnDestroy {
     } else {
       const styleEl = this.doc.createElement('style');
       styleEl.textContent = style;
-
-      if (this.platformIsServer) {
-        // This `ng-app` attribute is only required when server rendering for style re-using
-        // purposes.
-        styleEl.setAttribute('ng-app', this.appId);
-      }
+      styleEl.setAttribute(APP_ID_ATTRIBUTE_NAME, this.appId);
 
       return styleEl;
     }
diff --git a/packages/platform-browser/test/dom/shared_styles_host_spec.ts b/packages/platform-browser/test/dom/shared_styles_host_spec.ts
@@ -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>a {};</style>');
+      expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
     });
 
     it('should add new styles to hosts', () => {
       ssh.addHost(someHost);
       ssh.addStyles(['a {};']);
-      expect(someHost.innerHTML).toEqual('<style>a {};</style>');
+      expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
     });
 
     it('should add styles only once to hosts', () => {
       ssh.addStyles(['a {};']);
       ssh.addHost(someHost);
       ssh.addStyles(['a {};']);
-      expect(someHost.innerHTML).toEqual('<style>a {};</style>');
+      expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
     });
 
     it('should use the document head as default host', () => {
@@ -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>a {};</style>');
+      expect(someHost.innerHTML).toEqual('<style ng-app-id="app-id">a {};</style>');
 
       ssh.ngOnDestroy();
       expect(someHost.innerHTML).toEqual('');
diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts
@@ -556,7 +556,7 @@ describe('platform-server integration', () => {
        });
      }));
 
-  it('adds styles with ng-app attribute', waitForAsync(() => {
+  it('adds styles with ng-app-id attribute', waitForAsync(() => {
        const platform = platformDynamicServer([{
          provide: INITIAL_CONFIG,
          useValue: {document: '<html><head></head><body><app></app></body></html>'}
@@ -567,7 +567,7 @@ describe('platform-server integration', () => {
          const styles: any[] = head.children as any;
          expect(styles.length).toBe(1);
          expect(styles[0].textContent).toContain('color: red');
-         expect(styles[0].getAttribute('ng-app')).toBe('ng');
+         expect(styles[0].getAttribute('ng-app-id')).toBe('ng');
        });
      }));
 
