fix: possibility to override global services in Angular component tests

[rainerhahnekamp]

• Closes Angular Component Testing: providing of transitive dependencies fails #24047, Angular CT providers are not added to the TestingModule #23427

User facing changelog

Introduces componentProviders for Angular component testing which overrides a component's providers. Changes the existing providers so that they work exactly as the ones from TestModuleMetadata::providers.

Additional details

This PR introduces a new property componentProviders and is a breaking change. componentProviders overrides the providers of the component via the set attributes.

MountConfig::providers changes its behavior. It provides and overrides now services in the global scope. So it acts exactly in the same way, as native Angular Testing, Spectator, and Testing Library (Angular).

The name MountConfig:componentProviders was used in order to have a similar behavior as the existing testing libraries for Angular (Spectator, Testing Library).
Furthermore, I would argue that it should use the set property in TestBed.overrideComponent and not - as is currently the case - the add property. Again, this is to have full backwards-compatibility with the existing testing tools in Angular.

References:
https://github.com/ngneat/spectator/blob/master/projects/spectator/src/lib/spectator/create-factory.ts#L47
https://github.com/testing-library/angular-testing-library/blob/main/projects/testing-library/src/lib/testing-library.ts#L276

---

I would like to point out that the current behavior of MountConfig::providers wrongly implies that it is TestingModuleMetadata::providers and is, therefore, really confusing.

Steps to test

Run the system tests for angular component testing

How has the user experience changed?

No visual changes. Overriding services works as we know it from native Angular testing.

PR Tasks

• Have tests been added/updated?
• Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
• Has a PR for user-facing changes been opened in cypress-documentation?
• Have API changes been updated in the type definitions?

[cypress-bot]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

[CLAassistant]

All committers have signed the CLA.

[rainerhahnekamp]

@yjaaidi: this would be a fix for the providers issue we talked about

@astone123, @jordanpowell88: imho, this would make a much better developer experience without any workarounds

[jordanpowell88]

Thank you so much for the PR @rainerhahnekamp! This was work I was just getting ready to start!

[jordanpowell88]

Excellent job with this Rainer!

[ZachJW34]

I've changed the base-branch to target release/11.0.0 since this will be a breaking change. We plan on a v11 release next so this will be released soon once we get this merged up.

[ZachJW34]

Tested and it works great! Thanks @rainerhahnekamp for the contribution.

Changes look good, sorry for juggling your branch between develop and 11.0.0 I need to get 11.0.0 updated so that switching base branches doesn't bring along a bunch of other commits.

[ZachJW34]

I didn't know you could do this (rely on @Injectable({ providedIn: 'root' }) to inject into the TestModule). Really cool!

[ZachJW34]

Do you know the difference between add and set here? Hard to find documentation on this and I'm curious

[rainerhahnekamp]

Yeah, so with add, it is kind of a merge between the existing providers and the ones that you add in the test. With set, you replace them.

That's why you need to use set when you want to be able to remove all component providers (as the last test does)

[ZachJW34]

Thinking more on this, what is the behavior when specifying componentProviders when using the

cy.mount('<some-component />', { componentProviders: [SomeProvider], declarations: [SomeComponent] })

form? The providers would be attached to the WrapperComponent in this case, would SomeComponent inherit the providers?

[rainerhahnekamp]

It will inherit it, but if SomeComponent has its own providers, they will not be replaced. With a wrapper component it doesn't make any difference, if I use providers or componentProviders.

Theoretically, we could spend some effort trying to figure out that SomeComponent is embedded, but this is not an ideal solution. Just think of the following case:

cy.mount('<div><component-a></component-a><component-b></component-b></div>', {componentProviders: [SomeProvider]});

Now we have two components. To which one should we assign the SomeProvider?

The question is if it actually makes sense to provide componentProviders for a WrapperComponent. It might lead to further confusion.

Let me check, how other libraries deal with this situation and I'll come back to you.

What are your thoughts?

[ZachJW34]

I need to run through some test-cases and map out how the providers at any given level are able to be overridden. It's been a while since I've worked closely with Angular, but I'm starting to think we could cut componentProviders from the scope of this PR and move forward with the providers switch to be module-level. There might be a use-case for it in the future but I don't see the benefits of componentProviders clearly right now.

[rainerhahnekamp]

In Angular testing, you have two types of how to override providers.

The first is the providers of the TestBed.configureTestingModule, and the second one is the one in TestBed.overrideComponent. You need both.

With componentProviders you can override the providers inside the @Component({providers: ...}). providers of the TetsingModule will not reach them.

In my test, you see the difference between these two.

[rainerhahnekamp]

A typical use case for componentProviders is the @ngrx/component-store. This is a service, you usually provide inside the @Component({}): https://ngrx.io/guide/component-store/initialization#lazy-initialization

[yjaaidi]

Thanks @rainerhahnekamp for putting this together.

TL;DR: let's use add, instead of set.

This is in fact an unavoidable breaking change that would make less impact if added sooner than later.

The only thing though that I'd love to discuss here is the overrideComponent's set vs. add.
IMO, we should use add instead of set. As a matter of fact, add will override any providers anyway.
The problem with using set is that we'd have to provide all the required dependencies.

e.g. if a component depends on A & B and we just want to override A, we'd have to override B too... then when the component would add C, it would break test and we'd have to provide C too.
I think that using set makes tests more coupled to implementation details while add allows us only to override what we want which is the most common use case, I guess.

BTW, I'm advocating for providing a nicer TestBed without set or remove as I never found any advantage of using set or remove because I can't figure out a test where one would want to redefine all providers (while some of them might be implementation details like a presenter pattern or ephemeral state) or remove some...
so, I'm looking for any use case ƒor set or remove.

What do you think?

cc. @jordanpowell88 @ZachJW34

[yjaaidi]

TL;DR: componentProviders should not be available when using mount(template, ...).

Also, I just realized that we have to change the signature of mount as it should only allow componentProviders when used with a component class. If it's used with a template (string), we shouldn't allow the option in order to avoid any confusion.

As mentioned by @rainerhahnekamp, we don't need componentProviders when using templates... and if they are needed, then we'd need a lower level API that exposes something similar to TestBed where we can override other components but I don't think that this would be suitable.

[rainerhahnekamp]

Hi @yjaaidi,

I think in most cases you definitely want to use add instead set. But if it is add you cannot remove a provider anymore. This is shown in the test "should remove component-providers".

I think we have two options:
1: Choose between set or add
2: Don't choose but let the developer decide. So componentProviders would have the same signature as Metadata<Override>: https://angular.io/api/core/testing/MetadataOverride

In the PR I went for set because I wanted to have the compatibility to spectator and the testing-library (links in the first description).

[rainerhahnekamp]

@ZachJW34, I will remove the componentProviders for the mount command with the WrapperComponent. I hope that's fine with you.

[yjaaidi]

Hey @rainerhahnekamp, actually angular testing library is overriding existing providers using TestBed.overrideProvider which actually produces a similar behavior to add.

As we agree that add is the most common scenario (even though ppl should use providers at the test module level because componentProviders are clearly an implementation detail), we should provide the best DX for this use case then think about more hacky scenarios.

I know that even the Angular core team is not very happy with the MetadataOverride approach and they are planning some changes in the future but I think that nothing was decided yet.

👍 My favorite approach: I think that the best thing is to figure out a mix of your two suggestions:

// default behavior is `add`
// this is more type-safe than MetadataOverride as add, set and remove are mutually exclusive
componentProviders: Provider[] | {set: Provider[]} | {remove: Provider[]};

// e.g.
componentProviders: [FakeStore],
componentProviders: {set: []}; // remove all providers
componentProviders: {remove: [FakeStore]} // remove FakeStore provider only

🤔 Meh approach: Then, there is the functional approach which I find interesting because it's the one providing the most flexibility but it's bit overengineered and harder to maintain as it gets coupled to the way we fetch component's metadata.

componentProviders: Provider[] | (providers: Provider[]) => Provider[];

componentProviders: (providers) => [...providers.filter(provider => provider !== providerToRemove), myProvider]

what do you think?

[rainerhahnekamp]

@yjaaidi thanks for correcting me about the testing library. Didn't read the code carefully enough.

The componentProviders: Provider[] | {set: Provider[]} | {remove: Provider[]}; approach looks perfectly fine to me.

Let's see what our friends from Cypress think about it.

[yjaaidi]

Actually, we might also need add and make it coexist with remove.
TestBed.overrideComponent doesn't allow add and remove simultaneously but we can call TestBed.overrideComponent twice. One with remove then one with add if present.

The signature would become:

interface MountOptions {
  componentProviders: Provider[] | {set: Provider[]} | {add?: Provider[], remove?: Provider[]};
}

and allow the following:

mount(..., {componentProviders: {
  add: [Y, Z],
  remove: [X]
}})

where the implementation would something like this:

if ('remove' in componentProviders) {
  TestBed.overrideComponent({providers: {remove: ...}})
}
if ('add' in componentProviders) {
  TestBed.overrideComponent({providers: {add: ...}})
}

This might sound a bit far fetched as I can't figure out any decent test doing this but it's the most flexible approach offering all what you can do with TestBed and with better type safety.

[rainerhahnekamp]

@yjaaidi , @jordanpowell88, @ZachJW34, I need your help, please.

I used function overloading to exclude componentProviders when a template is used. Now the original JSDoc description for the mount function doesn't show up in the IDE anymore. Any suggestions on how to fix that?

[ZachJW34]

Thanks for all the conversation around this @rainerhahnekamp @yjaaidi, learning a lot here!

Before we get into the weeds about the set, add, or remove I'd like to discuss the inability to use componentProviders with cy.mount('<component-a></component-b>', { componentProviders: [...] }). From a user's perspective, I feel like there really shouldn't be a difference between the template and the class mount APIs (or rather, you should be able to accomplish anything with either form).

Before we move forward with making componentProviders only apply to the class form of the mount, @yjaaidi can you give me more information on your statement:

and if they are needed, then we'd need a lower level API that exposes something similar to TestBed where we can override other components but I don't think that this would be suitable.

@jordanpowell88 and I paired for a bit and started working through a solution that addresses this that I'd like to get feedback on. What I realized is that componentProviders is always one level-deep, meaning even when using the class form of the mount, if a child-component has component-level providers we offer no way of overriding them (which is the equivalent problem to why we can't have componentProviders for the template form).

If we changed the componentProviders model to allow passing in the ComponentType, we could then call

overrideComponent(ComponentType, { set/add: providers })

This would look roughly like:

cy.mount('<app-root></app-root>', {
  componentProviders: [{
    component: AppComponent,
    providers: [{ provide: MyService, useValue: { message: 'Hello World' } }],
  }],
  declarations: [AppComponent],
});

With this API, we could have an equivalency between the template and the component form. We could tweak the componentProviders type to have better DX or be dependent on the form you are using, but a user would be able to accomplish overriding component-level providers regardless of how they are mounted/rendered.

Thoughts?

[rainerhahnekamp]

I think we have to be a little bit careful here, not to overdo things.

Most of the component tests will not need the componentProviders at all, because you usually don't provide services in a component. The default way is to provide them globally via @Injectable({providedIn: 'root'}).

If we start now to tweak the mount function for edge cases, we have to tweak it for other edge cases as well. And they can be quite a lot if you take a look at the TestBed API: https://angular.io/api/core/testing/TestBed. overridePipe, overrideDirective...

It might be better to expose the TestBed for the edge cases and give the developers full control over it. Otherwise, you would have to write wrapper code for the complete API, which has to be maintained, etc.

If the TestBed would be available, we could even let go of the componentProviders. So from the developer experience, the usage of cy.mount would cover the majority of the use cases. For the edge cases, the developers can additionally use the TestBed.

That would be my thoughts...

[ZachJW34]

@rainerhahnekamp that's a fair point. The TestBed is available to use, it's just not very explicit but any user can import getTestBed and utilize it. It's hard to draw the line on what we should support via cy.mount and what the user can accomplish when given access to the TestBed. I mentioned removing componentProviders in a previous comment, and your point illustrates how we are only providing part of a solution since directives, imports, declarations and pipes also exist. I do wonder, with standalone components becoming more popular if these APIs aren't as edge-case as they once were.

[rainerhahnekamp]

I don't think standalone components will have any impact that might be relevant for this PR.

To cut it short, if we can all agree, that componentProviders are not needed because it can be done "manually" via the TestBed.overrideComponent(), then this PR is actually only about applying the providers property from cy.mount to TestBed.configureTestingModule.

If we can all agree on that, I would push the changes, and we are done.

Getting access to inject (see #24429) should probably be discussed in another PR.

Looking forward to @yjaaidi and @jordanpowell88 responses...

[jordanpowell88]

I think reverting providers to provide at the module level and removing componentProviders is the best option at the moment. We can try to also provide some additional documentation around how you can still use TestBed to leverage these edge cases to hopefully help users.

[yjaaidi]

Totally agreed with @jordanpowell88 the main goal here is to make providers add providers at the module level.

Then let's open an issue to think of a low level way of overriding more specific component providers.

Now, the problem is that this will be a breaking change where we don't provide any way of overriding providers at the component level. It might surprise some users.
What about adding overrideComponents for component mount only (not wrapper) and mark it as "preview" or "experimental" or something like that just meanwhile we figure out a low level API.

We have to figure out a homogeneous solution for this issue and these:

• Angular Component Testing: Allow standalone shallow testing by overriding component imports #24437
• Angular Component Testing: provide access to DI #24429

There are a couple of related questions that we have to think of:

• shouldn't we keep TestBed as an implementation detail?
• how do we provide a way to hook only into TestBed parts we want to expose?

Maybe a hook approach can solve this.

mount(..., {overrideHookWeHaveToName: ({overrideComponent, ...}) => {...}})

[rainerhahnekamp]

In my humble opinion, I see two steps here.

The first step is to make sure that Cypress 11 covers all testing use cases for Angular. In v10, this is not the case.

We do that via changing the providers (this PR) and highlighting in the documentation that the TestBed can be used directly for edge cases. This would also include overrideProviders, which would not be part of this PR.

---

The second step improves the developer experience. It would include all the issues, referenced by Younes.

Unlike the first step, we don’t have here any time pressure. Step 1 is a breaking change and should be merged as quickly as possible. I think step 2 is only about additional features, which we can add in any minor version.

I understand that the removal of componentProviders might be frustrating, but if we communicate the two-step strategy clearly, I think our fellow developers will agree to that.

---

I have added two tests that showcase the usage of TestBed.inject. I had to include it in the then method. It did not work outside. Maybe you find a better way.

Cheers!

[yjaaidi]

💯 agreed with @rainerhahnekamp
We should document that TeatBed is the temporary workaround.

Yup! Inject must be chained and executed only after the test module is set up. That's why I opened this issue #24429 in order to figure out an easier way of handling this

[yjaaidi]

Also, @rainerhahnekamp @jordanpowell88 @ZachJW34, we can release a new version of @jscutlery/cypress-angular which could be a wrapper of @cypress/angular in order to smoothen the DX.
You can see it as an incubator of ideas we can experiment there, then Cypress can absorb and adapt the best of them.
What do you think?

[ZachJW34]

Looks like this PR is good to go, I'll need to get a build triggered for this PR (looks like there is some permission issues in CircleCI that are preventing outside contributor builds) and then we can get this merged.

Thanks for all the feedback and creating the issues @rainerhahnekamp @yjaaidi! Using @jscutlery/cypress-angular to experiment with some APIs seems like a good idea to me so feel free!