testing-library/angular-testing-library

bug: `componentProviders` failed on nested array providers

Closed this issue · 4 comments

lacolaco commented

The providers in the Angular framework always flatten all elements in the case of nested arrays of elements. In other words, Provider can be an array of providers.

https://angular.dev/api/core/Provider

type Provider = | TypeProvider
  | ValueProvider
  | ClassProvider
  | ConstructorProvider
  | ExistingProvider
  | FactoryProvider
  | any[] // <--- an array of providers.

However, due to recent changes, ATL no longer accepts nested arrays in componentProviders property. Previously, array.concat(provider) had that role, but in the current implementation the step to flatten componentProviders is lost. This is a regresssion.

061d5cc#diff-8e35d38d4ce658063a1f7522f82a938a8b99a52223927129ad8ddd28df541506L124

-  componentProviders
-    .reduce((acc, provider) => acc.concat(provider), [] as any[])
-    .forEach((p: any) => {
-      const { provide, ...provider } = p;
-      TestBed.overrideProvider(provide, provider);
-    });
+  for (const { provide, ...provider } of componentProviders) {
+    TestBed.overrideProvider(provide, provider);
+  }
lacolaco commented

Strictly speaking, even array.concat(provider) would not be consistent with the Angular framework's behavior, as it would not be able to flatten nesting 2+ levels, but at least as a regression fix, componentProviders.flat() seems to be enough.

    for (const provider of componentProviders.flat()) {
        TestBed.overrideProvider(provide, provider);
    }
lacolaco commented

@timdeschryver How do you think the solution above; componentProviders.flat()? It has a lower risk than componentProviders.flat(Infinity) which can flatten nested arrays completely, but the incompatibility to Angular is not solved. IMO, componentProviders.flat(Infinity) is better.

timdeschryver commented

@lacolaco this is a case that we haven't thought of.
Feel free to create a PR using componentProviders.flat(Infinity), and maybe update providers as well?

Sided question: since standalone is the default, do you think it would be better to rename componentProviders to providers? And the existing providers to global/applicationProviders?

lacolaco commented

@lacolaco this is a case that we haven't thought of. Feel free to create a PR using componentProviders.flat(Infinity), and maybe update providers as well?

Sided question: since standalone is the default, do you think it would be better to rename componentProviders to providers? And the existing providers to global/applicationProviders?

@timdeschryver Yeah, I've created a PR. Please review it when you have time.
About the naming, I prefer the simple providers for @Component.providers. However, frequent API changes are a burden for ATL users, and since Angular itself has included improvements to TestBed in its roadmap, I think it is not too late to wait and see how that moves forward. I feel that it is not that big of a problem even in its current state.

  • 👍1