spectator

spectator copied to clipboard

PR Checklist

Please check if your PR fulfills the following requirements:

• [x] The commit message follows our guidelines: https://github.com/ngneat/spectator/blob/master/CONTRIBUTING.md#commit
• [ ] Tests for the changes have been added (for bug fixes / features)
• [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

For better or worse, the first invocation of TestBed.inject (or TestBed.get) prevents any further configuration of Providers.
The Spectator factories of course make TestBed.inject calls, but since the createXXXFactory functions also allow overrides for configuring Providers, Angular Dependency Injection can not be used by application testing code until after the Spectator factory has finished creating the entity under test and returned to it's caller.
In many cases this is okay, but if test code needs access to DI based resources before it can call a Spectator factory, then that user land code will have already invoked TestBed.inject and therefore the subsequent call to create the desired Spectator factory will fail. This typically manifests as a "Make sure you are not using inject before TestBed" error.

Issue Number: N/A

What is the new behavior?

This PR modifies createRoutingFactory to optionally avoid the TestBed.overrideProvider call during invocation of the factory by configuring the ActivatedRoute during creation of the factory.
It maintains backwards compatibility by checking during invocation of the factory to see if the previously configured ActivatedRoute needs to be overridden.

The TestBed constraint clearly prevents us from having it both ways (pre-factory access to DI, and overrides), but we can provide users the ability to choose one approach or the other. If you want to supply overrides to the spectator factory, you can't use DI beforehand. But if you require DI availability, then you can choose not to provide overrides to the factory.

This is the same general idea as PR #444 (i.e. Chose pre-factory DI, or override the template).

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

I've seen issues and discussions within this project regarding the inability to modify the TestBed after a call to TestBed.compileComponents. It seems that one can indeed modify Providers after TestBed.compileComponents (because this PR works fine). But it looks like you can't make component related modifications after compileComponents. That was a long winded way of asking if you would be interested in an additional PR documenting some of the configuration nuances between Spectator and TestBed? Since it is clearly a point of confusion for some of us. I'm not a guru, but I'd be happy to do the initial work as long a more experienced person would be willing to review it 😄

Working with this a little more, I'm wondering if:

      TestBed.overrideProvider(ActivatedRoute, {
        useValue: new ActivatedRouteStub({ params, queryParams, data, fragment, url, root, parent, children, firstChild })
      });

should have been wrapped in a if (overrides.stubsEnabled) block even in master ?

Have I misunderstood the stubsEnabled flag or should I open an issue for this ?

What is the issue you are trying to resolve? Can you please reproduce it first?

I was simply trying to call TestBed.inject after the call to create a Spectator factory, but before calling the factory itself.

const factory = createRoutingFactory({...});
const generator = TestBed.inject(MyDataGeneratorService);
const spectator = factory({props: {model: generator.generate()}});

That fails because the invocation of factory attempts to modify the TestBed which has already been sealed by the TestBed.inject call.

This pattern works for createComponentFactory, and with the PR I previously provided for createHostFactory, it will also work.

This PR was to enable the same pattern with createRoutingFactory.

However, I've decided to go in a different direction. So if you don't feel this functionality is useful, feel free to close this PR.

P.S. I do still think there is a defect in createRoutingFactory with regard to creating a stub even when overrides.stubsEnabled is false.