HTTP_INTERCEPTORS are reset when a lazy loaded module imports another module importing HttpClientModule

[thomashilzendegen]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

The HTTP_INTERCEPTORS provider token is reset when a lazy loaded module imports another module which imports the HttpClientModule by itself.

Expected behavior

The HTTP_INTERCEPTORS from the parent injector should be used.

Minimal reproduction of the problem with instructions

https://plnkr.co/edit/up8P57jwD3lUAZ9WyMS4

What is the motivation / use case for changing the behavior?

It seems that this is currently by design that a parent injector is not asked for the already registered providers from a multi-provider token. In my opinion it is not uncommon to have some kind of shared module which is also importing the HttpClientModule (because it want's to issue HTTP calls, but should not depend on having the module imported in the app module).

Is there any other way to implement this scenario?

Likely relates also to #18894 in some kind.

Environment

Angular version: 5.0.2

[abdel-ships-it]

Before you can use the HttpClient, you need to install the HttpClientModule which provides it. This can be done in your application module, and is only necessary once.

You should import the HttpClientModule only once, see docs

[thomashilzendegen]

@realappie Sure, but this is quite uncommon that the application module imports a module which has no forRoot() method just because another module uses it (which could be some kind of library etc.). I would expect that this module imports the HttpClientModule by itself and does not depend on a one-time import in the application module.

[mlc-mlapis]

@thomashilzendegen ... then HttpClientModule is loaded on lazy module level ... and without forRoot() it will be instantiated for each lazy loaded module = reset from your point of view.

[thomashilzendegen]

@mlc-mlapis Yep, that's true, but there is no forRoot() - so is it a bug or is it intended? And is there any other solution for that, beside You must not import HttpClientModule anywhere else but in the app module? In that case it should also be documented in that way.

[mlc-mlapis]

@thomashilzendegen ... actually it is intended ... because in 99% cases when you need AJAX communication with a server you probably need it also directly in your main app ... and not only in one or two lazy loaded modules.

But you are right that HttpClientModule with forRoot() would be more general ... so you can create a new request for a new feature ... but in the actual moment I am not sure about the level of a priority ...

[thomashilzendegen]

@mlc-mlapis The funny thing is that this breaks currently, I have the HttpClientModule in my app module (see the Plunker), but when a lazy loaded module imports a module that imports the HttpClientModule, too, the interceptors are gone. This is because that multi-providers don't look at the injector inheritance, I guess.

[svenhuber]

@thomashilzendegen same here

[macjohnny]

@mlc-mlapis @realappie at least the docs should be updated to warn about importing the HttpClientModule more than once, e.g. also in a lazy-loaded module, and add a section about this issue as a possible reason why an interceptor might not be loaded.

[thomashilzendegen]

@macjohnny Actually, it isn't imported in the lazy-loaded module directly. The lazy-loaded module just uses a module which imports the HttpClientModule. And I think this is the dangerous part, you may not know that this module imports the HttpClientModule when its not your code, for example (without having a look).

[mlc-mlapis]

@thomashilzendegen, @macjohnny ... a standard structure is to access HttpClientModule only through DI. There isn't probably actually any real case to access it by any other way and repeat the importing. I don't know any, at least.

[macjohnny]

@mlc-mlapis i am sorry but i can‘t follow your argument.
I think importing some module that itself imports the HttpClientModule is a valid use case.
In fact we faced exactly this situation, which is why we found this issue...

[mlc-mlapis]

@macjohnny ... it could happen of course but still ... can you describe that scenario a bit to understand how it was happened as a real case?

[macjohnny]

@mlc-mlapis in our app module we added a provider for our custom interceptor.
In our lazy-loaded feature module we imported the HttpClientModule, and in result our custom interceptor was not provided in the feature module.
The solition was to remove the httpclientmodule import in the feature module.
As @thomashilzendegen stated above, the situation is even harder to figure out when importing some other module in the festure module, while this other module imports the HttpClientModule

[mlc-mlapis]

@macjohnny ... thanks. Yes, it is a hidden = that's why a dangerous part. It should be notified more explicitly in the doc.

But from the app architecture I still can't see any reason to import HttpClientModule except the main app module and always use DI in any other module. So importing the module somewhere else is probably a mistake from 99%.

[macjohnny]

@mlc-mlapis i agree that it is a mistake to import the httpclientmodule in any module other than the app module, however, providing a forRoot() method with all providers would prevent this situation. Alternatively, the httpclientmodule could throw an error, since, as you stated, it is a mistake. But silently allowong it withiut any warning can lead to hard-to-find effects.
Especially if you are including e.g. A library which imports the httpclientmodule, it is dangerous.