fix(core): prevent BrowserModule providers from being loaded twice

goldens/public-api/platform-browser/index.md

@@ -33,7 +33,7 @@ export function bootstrapApplication(rootComponent: Type<unknown>, options?: App
 
 // @public
 export class BrowserModule {
-    constructor(parentModule: BrowserModule | null);
+    constructor(providersAlreadyPresent: boolean | null);
     static withServerTransition(params: {
         appId: string;
     }): ModuleWithProviders<BrowserModule>;

packages/core/test/bundling/animations/bundle.golden_symbols.json

@@ -74,6 +74,9 @@
   {
     "name": "BROWSER_MODULE_PROVIDERS"
   },
+  {
+    "name": "BROWSER_MODULE_PROVIDERS_MARKER"
+  },
   {
     "name": "BROWSER_NOOP_ANIMATIONS_PROVIDERS"
   },

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

@@ -38,6 +38,9 @@
   {
     "name": "BROWSER_MODULE_PROVIDERS"
   },
+  {
+    "name": "BROWSER_MODULE_PROVIDERS_MARKER"
+  },
   {
     "name": "BaseControlValueAccessor"
   },

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

@@ -41,6 +41,9 @@
   {
     "name": "BROWSER_MODULE_PROVIDERS"
   },
+  {
+    "name": "BROWSER_MODULE_PROVIDERS_MARKER"
+  },
   {
     "name": "BaseControlValueAccessor"
   },

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -44,6 +44,9 @@
   {
     "name": "BROWSER_MODULE_PROVIDERS"
   },
+  {
+    "name": "BROWSER_MODULE_PROVIDERS_MARKER"
+  },
   {
     "name": "BehaviorSubject"
   },

packages/platform-browser/src/browser.ts

@@ -7,7 +7,7 @@
  */
 
 import {CommonModule, DOCUMENT, XhrFactory, ɵPLATFORM_BROWSER_ID as PLATFORM_BROWSER_ID} from '@angular/common';
-import {APP_ID, ApplicationModule, ApplicationRef, createPlatformFactory, ErrorHandler, ImportedNgModuleProviders, Inject, ModuleWithProviders, NgModule, NgZone, Optional, PLATFORM_ID, PLATFORM_INITIALIZER, platformCore, PlatformRef, Provider, RendererFactory2, SkipSelf, StaticProvider, Testability, Type, ɵbootstrapApplication as _bootstrapApplication, ɵINJECTOR_SCOPE as INJECTOR_SCOPE, ɵsetDocument} from '@angular/core';
+import {APP_ID, ApplicationModule, ApplicationRef, createPlatformFactory, ErrorHandler, ImportedNgModuleProviders, Inject, InjectionToken, ModuleWithProviders, NgModule, NgZone, Optional, PLATFORM_ID, PLATFORM_INITIALIZER, platformCore, PlatformRef, Provider, RendererFactory2, SkipSelf, StaticProvider, Testability, Type, ɵbootstrapApplication as _bootstrapApplication, ɵINJECTOR_SCOPE as INJECTOR_SCOPE, ɵsetDocument} from '@angular/core';
 
 import {BrowserDomAdapter} from './browser/browser_adapter';
 import {SERVER_TRANSITION_PROVIDERS, TRANSITION_ID} from './browser/server-transition';
@@ -19,6 +19,8 @@ import {EVENT_MANAGER_PLUGINS, EventManager} from './dom/events/event_manager';
 import {KeyEventsPlugin} from './dom/events/key_events';
 import {DomSharedStylesHost, SharedStylesHost} from './dom/shared_styles_host';
 
+const NG_DEV_MODE = typeof ngDevMode === 'undefined' || !!ngDevMode;
+
 /**
  * Set of config options available during the bootstrap operation via `bootstrapApplication` call.
  *
@@ -96,17 +98,24 @@ export const INTERNAL_BROWSER_PLATFORM_PROVIDERS: StaticProvider[] = [
 export const platformBrowser: (extraProviders?: StaticProvider[]) => PlatformRef =
     createPlatformFactory(platformCore, 'browser', INTERNAL_BROWSER_PLATFORM_PROVIDERS);
 
+/**
+ * Internal marker to signal whether providers from the `BrowserModule` are already present in DI.
+ * This is needed to avoid loading `BrowserModule` providers twice. We can't rely on the
+ * `BrowserModule` presence itself, since the standalone-based bootstrap just imports
+ * `BrowserModule` providers without referencing the module itself.
+ */
+const BROWSER_MODULE_PROVIDERS_MARKER =
+    new InjectionToken(NG_DEV_MODE ? 'BrowserModule Providers Marker' : '');
+
 export const BROWSER_MODULE_PROVIDERS: StaticProvider[] = [
   {provide: INJECTOR_SCOPE, useValue: 'root'},
-  {provide: ErrorHandler, useFactory: errorHandler, deps: []},
-  {
+  {provide: ErrorHandler, useFactory: errorHandler, deps: []}, {
     provide: EVENT_MANAGER_PLUGINS,
     useClass: DomEventsPlugin,
     multi: true,
     deps: [DOCUMENT, NgZone, PLATFORM_ID]
   },
-  {provide: EVENT_MANAGER_PLUGINS, useClass: KeyEventsPlugin, multi: true, deps: [DOCUMENT]},
-  {
+  {provide: EVENT_MANAGER_PLUGINS, useClass: KeyEventsPlugin, multi: true, deps: [DOCUMENT]}, {
     provide: DomRendererFactory2,
     useClass: DomRendererFactory2,
     deps: [EventManager, DomSharedStylesHost, APP_ID]
@@ -117,6 +126,7 @@ export const BROWSER_MODULE_PROVIDERS: StaticProvider[] = [
   {provide: Testability, useClass: Testability, deps: [NgZone]},
   {provide: EventManager, useClass: EventManager, deps: [EVENT_MANAGER_PLUGINS, NgZone]},
   {provide: XhrFactory, useClass: BrowserXhr, deps: []},
+  NG_DEV_MODE ? {provide: BROWSER_MODULE_PROVIDERS_MARKER, useValue: true} : []
 ];
 
 /**
@@ -128,12 +138,17 @@ export const BROWSER_MODULE_PROVIDERS: StaticProvider[] = [
  *
  * @publicApi
  */
-@NgModule({providers: BROWSER_MODULE_PROVIDERS, exports: [CommonModule, ApplicationModule]})
+@NgModule({
+  providers: BROWSER_MODULE_PROVIDERS,
+  exports: [CommonModule, ApplicationModule],
+})
 export class BrowserModule {
-  constructor(@Optional() @SkipSelf() @Inject(BrowserModule) parentModule: BrowserModule|null) {
-    if (parentModule) {
+  constructor(@Optional() @SkipSelf() @Inject(BROWSER_MODULE_PROVIDERS_MARKER)
+              providersAlreadyPresent: boolean|null) {
+    if (NG_DEV_MODE && providersAlreadyPresent) {
       throw new Error(
-          `BrowserModule has already been loaded. If you need access to common directives such as NgIf and NgFor from a lazy loaded module, import CommonModule instead.`);
+          `Providers from the \`BrowserModule\` have already been loaded. If you need access ` +
+          `to common directives such as NgIf and NgFor, import the \`CommonModule\` instead.`);
     }
   }
 

packages/platform-browser/test/browser/bootstrap_spec.ts

@@ -449,7 +449,9 @@ function bootstrap(
                return compiler.compileModuleAsync(AsyncModule).then(factory => {
                  expect(() => factory.create(ref.injector))
                      .toThrowError(
-                         `BrowserModule has already been loaded. If you need access to common directives such as NgIf and NgFor from a lazy loaded module, import CommonModule instead.`);
+                         'Providers from the `BrowserModule` have already been loaded. ' +
+                         'If you need access to common directives such as NgIf and NgFor, ' +
+                         'import the `CommonModule` instead.');
                });
              })
              .then(() => done(), err => done.fail(err));

packages/platform-browser/test/browser/bootstrap_standalone_spec.ts

@@ -10,7 +10,7 @@ import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
 import {Component, destroyPlatform, Inject, Injectable, InjectionToken, NgModule} from '@angular/core';
 import {inject} from '@angular/core/testing';
 
-import {bootstrapApplication} from '../../src/browser';
+import {bootstrapApplication, BrowserModule} from '../../src/browser';
 
 describe('bootstrapApplication for standalone components', () => {
   let rootEl: HTMLUnknownElement;
@@ -21,7 +21,7 @@ describe('bootstrapApplication for standalone components', () => {
 
   afterEach(() => {
     destroyPlatform();
-    rootEl.remove();
+    rootEl?.remove();
   });
 
   it('should create injector where ambient providers shadow explicit providers', async () => {
@@ -108,4 +108,60 @@ describe('bootstrapApplication for standalone components', () => {
          expect((e as Error).message).toContain('No provider for InjectionToken ambient token!');
        }
      });
+
+  it('should throw if `BrowserModule` is imported in the standalone bootstrap scenario',
+     async () => {
+       @Component({
+         selector: 'test-app',
+         template: '...',
+         standalone: true,
+         imports: [BrowserModule],
+       })
+       class StandaloneCmp {
+       }
+
+       try {
+         await bootstrapApplication(StandaloneCmp);
+
+         // The `bootstrapApplication` already includes the set of providers from the
+         // `BrowserModule`, so including the `BrowserModule` again will bring duplicate providers
+         // and we want to avoid it.
+         fail('Expected to throw');
+       } catch (e: unknown) {
+         expect(e).toBeInstanceOf(Error);
+         expect((e as Error).message)
+             .toContain('Providers from the `BrowserModule` have already been loaded.');
+       }
+     });
+
+  it('should throw if `BrowserModule` is imported indirectly in the standalone bootstrap scenario',
+     async () => {
+       @NgModule({
+         imports: [BrowserModule],
+       })
+       class SomeDependencyModule {
+       }
+
+       @Component({
+         selector: 'test-app',
+         template: '...',
+         standalone: true,
+         imports: [SomeDependencyModule],
+       })
+       class StandaloneCmp {
+       }
+
+       try {
+         await bootstrapApplication(StandaloneCmp);
+
+         // The `bootstrapApplication` already includes the set of providers from the
+         // `BrowserModule`, so including the `BrowserModule` again will bring duplicate providers
+         // and we want to avoid it.
+         fail('Expected to throw');
+       } catch (e: unknown) {
+         expect(e).toBeInstanceOf(Error);
+         expect((e as Error).message)
+             .toContain('Providers from the `BrowserModule` have already been loaded.');
+       }
+     });
 });