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

HttpClientModule Module Issue #164

Closed
naveedahmed1 opened this issue Oct 11, 2017 · 10 comments
Closed

HttpClientModule Module Issue #164

naveedahmed1 opened this issue Oct 11, 2017 · 10 comments

Comments

@naveedahmed1
Copy link

I'm submitting a ... (check one with "x")

[x ] bug report => search for a similar issue before submitting
[ ] feature request
[ ] question

Versions

Beta 4

Repro steps

Since, you have imported HttpClientModule in the plugin, it now creates a new instance of HttpClient in the modules where we add this plugin. This breaks the application flow, for example for the cases where we have implemented HttpInterceptor to add custom headers to the requests.

A better approach would be, not to import HttpClientModule in the plugin. And in documentation ask the users to import the HttpClientModule in their app root module. This way we can avoid the multiple instances of the HttpClient.

@MurhafSousli
Copy link
Owner

MurhafSousli commented Oct 11, 2017

Hi @naveedahmed1, that's right, I forgot to include HttpClient in rollup.
Use npm install ngx-sharebuttons@4.0.0-beta for now. I am pushing a stable update very soon.

@naveedahmed1
Copy link
Author

Thanks! @MurhafSousli for the quick reply.

@naveedahmed1
Copy link
Author

naveedahmed1 commented Oct 14, 2017

@MurhafSousli sorry those comments should be on this issue. I remember I already had an open issue regarding this :D

I think the issue still exists. Since you have added HttpClient in the providers, that's why its creating another instance of HttpClient. Have a look at hierarchical nature of dependency injection in Angular https://angular.io/guide/hierarchical-dependency-injection

Let me rephrase the issue, when building angular apps that requires http requests, we add HttpClientModuleto the imports of the AppModule. Similarly, when we need to perform some operations before sending requests or after the response has been received. We use Interceptors, and add it to the Providers of the AppModule.

The reason we add, HttpClientModule to the imports of the AppModule and Interceptor to the Providers of AppModule is to have a single instance of HttpClient and Interceptor.

If we add HttpClientModule to any other module it will create another instance of HttpClient.

Now the issue is, when we add ShareButtonsModule to the imports of a LazyLoaded module, the providers of the ShareButtonsModule are merged with providers of LazyLoaded module, as a results HttpClient from providers of ShareButtonsModule is now in the providers of the LazyLoaded module hence resulting in a new instance of HttpClient scoped. The side effect of this is the Interceptor added in the AppModule has no effect on this new instance of the HttpClient since its scopped at LazyLoaded module only.

Please reopen this issue, since it has not been resolved yet.

@naveedahmed1
Copy link
Author

Can you please try removing HttpClient from this line deps: [HttpClient, OPTIONS, BUTTONS_META] ? Also remove HttpClientModule, HttpClientJsonpModule from imports array.

@MurhafSousli
Copy link
Owner

@naveedahmed1 Can you reproduce your issue in a repo so I can see what is happening!

@naveedahmed1
Copy link
Author

Just add the below HttpInterceptor to the Providers of the AppModule. Send http request from App component and view headers of the outgoing request from Chrome developer tools > Network tab, don't add ShareButtonsModule to AppModule.

import { Injectable } from '@angular/core';
import { HttpEvent, HttpInterceptor, HttpHandler, HttpRequest, HttpResponse, HttpErrorResponse, HttpEventType } from '@angular/common/http';

import { Observable } from 'rxjs/Observable';

@Injectable()
export class MyInterceptor implements HttpInterceptor {

    constructor() { }

    intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
        console.log('intercepted request to add authorization headers');
        let modifiedRequest: any;
        let newHeaders = req.headers.set('Authorization', 'Bearer fkdjfdsljfldsjfdsljflgjdflgjljgdflkgjflk');
        modifiedRequest = req.clone({ headers: newHeaders, url: req.url, body: req.body });
        return next.handle(modifiedRequest);
    }
}

Add a lazyloaded module and import ShareButtonsModule in this module. From a component of thisLazyloaded module send Http request and monitor its header.

You will notice that the request sent from AppComponent will contain authorization headers and a log on console. Whereas request sent from LazyComponent wont have it.

@MurhafSousli
Copy link
Owner

Reproduce the issue with this stackblitz

@naveedahmed1
Copy link
Author

@MurhafSousli please take a look at the below link:

https://stackblitz.com/edit/angular-fpfsyp?file=app%2Flazy.component.html

Press F12 to launch Developer tools and reload page.

Switch to Network tab and monitors the header added to the outgoing requests.

When request is sent from Home component, it will contains below headers which are added from interceptor:

Authorization:Bearer Test authorization token fkdjfdsljfldsjfdsljflgjdflgjljgdflkgjflk

Whereas, when request is sent from Lazy component, which contains Sahre buttons plugin, it wont have above headers, since its now being sent from another instance of HttpClient.

@MurhafSousli
Copy link
Owner

@naveedahmed1 Here is a fixed stackblitz

You can make a CustomHttpModule to avoid importing the interceptors each time, then import it in your lazy module

dup #174

@naveedahmed1
Copy link
Author

Thanks for sharing this, I think you missed my point. My concern is importing this way we would end up with multiple instances of HttpClient.

Check the updated link: https://stackblitz.com/edit/angular-vz4hjg?file=app/my-interceptor.ts

Monitor the console log, I have updated the interceptor to assign an id to the instance, and print on console.

sending request through instance with id: [id]

I would suggest removing the HttpClient dependency completely. Ask users to add HttpClient to the providers array of their root module.

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

No branches or pull requests

2 participants