Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(createRoutingFactory): Support use of DI prior to calling createRoutingFactory #475

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pcafstockf
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

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 😄

@pcafstockf
Copy link
Contributor Author

pcafstockf commented Aug 25, 2021

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 ?

@NetanelBasal
Copy link
Member

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

@pcafstockf
Copy link
Contributor Author

pcafstockf commented Aug 27, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants