fix(compiler): TestBed.overrideProvider should keep imported NgModules eager (#19624)

Before, as soon as a user called `TestBed.overrideProvider` for a provider
of a `NgModule` that was imported via `TestBed.configureTestingModule`,
that `NgModule` became lazy.

This commit changes this behavior to keep the `NgModule` eager,
with or without a call to `TestBed.overrideProvider`.

PR Close #19624

packages/core/src/view/services.ts

@@ -209,9 +209,6 @@ function applyProviderOverridesToView(def: ViewDefinition): ViewDefinition {
         return;
       }
       if (nodeDef.flags & NodeFlags.CatProviderNoDirective) {
-        // Make all providers lazy, so that we don't get into trouble
-        // with ordering problems of providers on the same element
-        nodeDef.flags |= NodeFlags.LazyProvider;
         const provider = nodeDef.provider !;
         const override = providerOverrides.get(provider.token);
         if (override) {
@@ -228,7 +225,8 @@ function applyProviderOverridesToView(def: ViewDefinition): ViewDefinition {
 // We only create new datastructures if we need to, to keep perf impact
 // reasonable.
 function applyProviderOverridesToNgModule(def: NgModuleDefinition): NgModuleDefinition {
-  if (providerOverrides.size === 0 || !hasOverrrides(def)) {
+  const {hasOverrides, hasDeprecatedOverrides} = calcHasOverrides(def);
+  if (!hasOverrides) {
     return def;
   }
   // clone the whole view definition,
@@ -237,18 +235,32 @@ function applyProviderOverridesToNgModule(def: NgModuleDefinition): NgModuleDefi
   applyProviderOverrides(def);
   return def;
 
-  function hasOverrrides(def: NgModuleDefinition): boolean {
+  function calcHasOverrides(def: NgModuleDefinition):
-    return def.providers.some(
+      {hasOverrides: boolean, hasDeprecatedOverrides: boolean} {
-        node =>
+    let hasOverrides = false;
-            !!(node.flags & NodeFlags.CatProviderNoDirective) && providerOverrides.has(node.token));
+    let hasDeprecatedOverrides = false;
+    if (providerOverrides.size === 0) {
+      return {hasOverrides, hasDeprecatedOverrides};
+    }
+    def.providers.forEach(node => {
+      const override = providerOverrides.get(node.token);
+      if ((node.flags & NodeFlags.CatProviderNoDirective) && override) {
+        hasOverrides = true;
+        hasDeprecatedOverrides = hasDeprecatedOverrides || override.deprecatedBehavior;
+      }
+    });
+    return {hasOverrides, hasDeprecatedOverrides};
   }
 
   function applyProviderOverrides(def: NgModuleDefinition) {
     for (let i = 0; i < def.providers.length; i++) {
       const provider = def.providers[i];
-      // Make all providers lazy, so that we don't get into trouble
+      if (hasDeprecatedOverrides) {
-      // with ordering problems of providers on the same element
+        // We had a bug where me made
-      provider.flags |= NodeFlags.LazyProvider;
+        // all providers lazy. Keep this logic behind a flag
+        // for migrating existing users.
+        provider.flags |= NodeFlags.LazyProvider;
+      }
       const override = providerOverrides.get(provider.token);
       if (override) {
         provider.flags = (provider.flags & ~NodeFlags.CatProviderNoDirective) | override.flags;

packages/core/src/view/types.ts

@@ -506,6 +506,7 @@ export interface ProviderOverride {
   flags: NodeFlags;
   value: any;
   deps: ([DepFlags, any]|any)[];
+  deprecatedBehavior: boolean;
 }
 
 export interface Services {

packages/core/testing/src/test_bed.ts

@@ -467,7 +467,7 @@ export class TestBed implements Injector {
       }
       return [depFlags, depToken];
     });
-    overrideProvider({token, flags, deps, value});
+    overrideProvider({token, flags, deps, value, deprecatedBehavior: deprecated});
   }
 
   createComponent<T>(component: Type<T>): ComponentFixture<T> {

packages/platform-browser/test/testing_public_spec.ts

@@ -468,6 +468,46 @@ export function main() {
             expect(modFactory.create(getTestBed()).injector.get('a')).toBe('mockA: parentDepValue');
           });
 
+          it('should keep imported NgModules eager', () => {
+            let someModule: SomeModule|undefined;
+
+            @NgModule()
+            class SomeModule {
+              constructor() { someModule = this; }
+            }
+
+            TestBed.configureTestingModule({
+              providers: [
+                {provide: 'a', useValue: 'aValue'},
+              ],
+              imports: [SomeModule]
+            });
+            TestBed.overrideProvider('a', {useValue: 'mockValue'});
+
+            expect(TestBed.get('a')).toBe('mockValue');
+            expect(someModule).toBeAnInstanceOf(SomeModule);
+          });
+
+          it('should keep imported NgModules lazy with deprecatedOverrideProvider', () => {
+            let someModule: SomeModule|undefined;
+
+            @NgModule()
+            class SomeModule {
+              constructor() { someModule = this; }
+            }
+
+            TestBed.configureTestingModule({
+              providers: [
+                {provide: 'a', useValue: 'aValue'},
+              ],
+              imports: [SomeModule]
+            });
+            TestBed.deprecatedOverrideProvider('a', {useValue: 'mockValue'});
+
+            expect(TestBed.get('a')).toBe('mockValue');
+            expect(someModule).toBeUndefined();
+          });
+
           describe('injecting eager providers into an eager overwritten provider', () => {
             @NgModule({
               providers: [