fix(core): makeEnvironmentProviders should accept EnvironmentProviders (#48720)

alxhub · AndrewKushnir · commit f00bf7141101 · 2023-01-12T13:47:59.000-08:00
`makeEnvironmentProviders` constructs the wrapped `EnvironmentProviders` type, which can only be used in environment injectors (not element injectors). It makes sense that `makeEnvironmentProviders` should be able to accept existing `EnvironmentProviders`-wrapped providers, since it will be providing the same guarantee, but the current types do not allow this. This commit fixes the typings to allow nesting `EnvironmentProviders` and adds a test to verify that it will work. PR Close #48720
diff --git a/goldens/public-api/core/index.md b/goldens/public-api/core/index.md
@@ -884,7 +884,7 @@ export class KeyValueDiffers {
 export const LOCALE_ID: InjectionToken<string>;
 
 // @public
-export function makeEnvironmentProviders(providers: Provider[]): EnvironmentProviders;
+export function makeEnvironmentProviders(providers: (Provider | EnvironmentProviders)[]): EnvironmentProviders;
 
 // @public
 export enum MissingTranslationStrategy {
diff --git a/packages/core/src/di/interface/provider.ts b/packages/core/src/di/interface/provider.ts
@@ -352,7 +352,7 @@ export type EnvironmentProviders = {
 };
 
 export interface InternalEnvironmentProviders extends EnvironmentProviders {
-  ɵproviders: Provider[];
+  ɵproviders: (Provider|EnvironmentProviders)[];
 
   /**
    * If present, indicates that the `EnvironmentProviders` were derived from NgModule providers.
@@ -362,8 +362,9 @@ export interface InternalEnvironmentProviders extends EnvironmentProviders {
   ɵfromNgModule?: true;
 }
 
-export function isEnvironmentProviders(value: Provider|InternalEnvironmentProviders):
-    value is InternalEnvironmentProviders {
+export function isEnvironmentProviders(
+    value: Provider|EnvironmentProviders|
+    InternalEnvironmentProviders): value is InternalEnvironmentProviders {
   return value && !!(value as InternalEnvironmentProviders).ɵproviders;
 }
 
diff --git a/packages/core/src/di/provider_collection.ts b/packages/core/src/di/provider_collection.ts
@@ -28,7 +28,8 @@ import {INJECTOR_DEF_TYPES} from './internal_tokens';
  * Wrap an array of `Provider`s into `EnvironmentProviders`, preventing them from being accidentally
  * referenced in `@Component in a component injector.
  */
-export function makeEnvironmentProviders(providers: Provider[]): EnvironmentProviders {
+export function makeEnvironmentProviders(providers: (Provider|EnvironmentProviders)[]):
+    EnvironmentProviders {
   return {
     ɵproviders: providers,
   } as unknown as EnvironmentProviders;
diff --git a/packages/core/src/di/r3_injector.ts b/packages/core/src/di/r3_injector.ts
@@ -516,8 +516,7 @@ function couldBeInjectableType(value: any): value is ProviderToken<any> {
 }
 
 function forEachSingleProvider(
-    providers: Array<Provider|InternalEnvironmentProviders>,
-    fn: (provider: SingleProvider) => void): void {
+    providers: Array<Provider|EnvironmentProviders>, fn: (provider: SingleProvider) => void): void {
   for (const provider of providers) {
     if (Array.isArray(provider)) {
       forEachSingleProvider(provider, fn);
diff --git a/packages/core/test/acceptance/di_spec.ts b/packages/core/test/acceptance/di_spec.ts
@@ -24,7 +24,12 @@ const collectEnvironmentInitializerProviders = (providers: Provider[]) =>
     getProvidersByToken(providers, ENVIRONMENT_INITIALIZER);
 
 function unwrappedImportProvidersFrom(...sources: ImportProvidersSource[]): Provider[] {
-  return (importProvidersFrom(...sources) as unknown as InternalEnvironmentProviders).ɵproviders;
+  const providers =
+      (importProvidersFrom(...sources) as unknown as InternalEnvironmentProviders).ɵproviders;
+  if (providers.some(provider => 'ɵproviders' in provider)) {
+    throw new Error(`Unexpected nested EnvironmentProviders in test`);
+  }
+  return providers as Provider[];
 }
 
 describe('importProvidersFrom', () => {
@@ -343,6 +348,14 @@ describe('EnvironmentProviders', () => {
     expect(inj.get(TOKEN)).toEqual('token!');
   });
 
+  it('should be accepted as additional input to makeEnvironmentProviders', () => {
+    const wrappedProviders = makeEnvironmentProviders([environmentProviders]);
+    TestBed.configureTestingModule({});
+
+    const inj = createEnvironmentInjector([wrappedProviders], TestBed.inject(EnvironmentInjector));
+    expect(inj.get(TOKEN)).toEqual('token!');
+  });
+
   it('should be overridable by TestBed overrides', () => {
     TestBed.configureTestingModule({
       providers: [environmentProviders],

Original file line number	Diff line number	Diff line change
`@@ -516,8 +516,7 @@ function couldBeInjectableType(value: any): value is ProviderToken<any> {`
`516`	`516`	`}`
`517`	`517`
`518`	`518`	`function forEachSingleProvider(`
`519`		`- providers: Array<Provider\|InternalEnvironmentProviders>,`
`520`		`- fn: (provider: SingleProvider) => void): void {`
	`519`	`+ providers: Array<Provider\|EnvironmentProviders>, fn: (provider: SingleProvider) => void): void {`
`521`	`520`	`for (const provider of providers) {`
`522`	`521`	`if (Array.isArray(provider)) {`
`523`	`522`	`forEachSingleProvider(provider, fn);`