I'm getting this error on my app. Looks like the library is requiring me to import the HttpModule by myself and configure it in my own app module.
Shouldn't be better for you guys to import it in your own library module so it's imported along with your module?
I'm trying to get rid of HttpModule in favor of the new HttpClientModule which is said to replace it. Perhaps you should do the same?
sounds like a good idea,
@robisim74 @FabianGosebrink ?
Yes, but this would be a breaking change. It has to be well communicated.
However this Issue is targeting two things. First, the upgrade of HttpModule to HttpClientModule. Second the re-export of the HttpClientModule in the Lib. Shall we do both?
Just a question: when you say breaking change are you referring from the consumer point of view or otherwise?
Because if you import another module from within your library, I guess it should be transparent for me.
Anyway, my main point on this issue was about your lib importing whatever module you want to implement it with and then have that module re-exported so I don't have to tinker around with the modules on my end.
IMHO we should use the new HttpClient (other libs do this as well).
@robisim74 @damienbod what do you think about the re-export?
About re-export: I don't like it, and it is not a best practice for an Angular library, because you create a hidden dependency, and probably in the host application the module will be re-imported again by who does not know the library internally.
About HttpClient: it is a _simplified API for HTTP_, just to work with json. HttpModule is not deprecated. We can switch to use it, but I agree with Fabian, it would be a breaking change: for a new major version perhaps it would be better to wait for the release of Angular v5. But keep in mind that we will not solve the problem, because who is using HttpModule, then will tell us: "No provider for HttpClient!".
Ok, but aside of the re-export or the change from HttpModule to HttpClientModule (which would be 2 things totally optional and at your discretion), why not doing the import of HttpModule directly from your lib module? Why requiring the consumer app to explicitly need to declare something that might not be using at all? Or am I missing something? (I'm fairly new in Angular 4, so I might :) )
Since Angular has been built as a library suite (modules in the ES6 syntax), external libraries (but also the same Angular) use dependencies as peer dependences, leaving the host application to import them. Otherwise, more imports of the same module and different versions could occur, which would break the application.
So, it would be a bad practice to do what I mentioned? Because I understood that was a good idea based on the Angular NgModule's guide: https://angular.io/guide/ngmodule.
Would this conflict with using dependencies as "peer dependencies", given that you use Http service internally and is not exposed at all?
This is a library, not an app. I try to explain with an example:
A dev builds an app importing HttpModule (or HttpClient is the same), for example using the latest version (4.3.5) because it needs it.
Then he installs this library: at the moment, this library uses the HttpModule installed from the host app (because it requires a peer dependencies of HttpModule ^4.1.0) and the app works without problems.
Instead, if this library had its own HttpModule 4.1.0 dependency, the app would have a conflict of module versions and a double import that could break the app.
Yes, I understand this being a library, but -from what I read in articles and q&a's like SO: (https://stackoverflow.com/a/34645112 provides 2 good examples)- wouldn't what you say apply if you were exposing Http somehow? I could bundle 2 different versions of the same library as far as you don't force me to "send you" something with the same version you're using instead of mine (unless you guys are expecting such scenario in the future).
I haven't made a proof of concept for this, only what I read, so please correct me if I'm wrong. And thanks a lot for your explanations. Much appreciated.
@CesarD I complete this reasoning with the main example:
Try to install only the official @angula/common, and you will see: @angula/common requires a peer dependency of @angular/core etc.
Then it is your responsibility to install @angular/core and import it in your AppModule.
In the same way, this library requires:
"peerDependencies": {
"@angular/core": ">= 4.1.0",
"@angular/http": ">= 4.1.0",
"@angular/router": ">= 4.1.0"
},
and that you import them in your AppModule:
@NgModule({
imports: [
...
RouterModule.forRoot(...),
HttpModule,
AuthModule.forRoot()
],
...
})
The Angular ecosystem works well in this way. Other ways create a lot of Uncaught Error: Unexpected value ... imported by the module etc. and a lot of time wasted to find the problem.
Greetings
Hmm, I'm a bit confused: while I can understand having peer dependencies to not create version conflicts, in the case of BrowserModule (from @angular/platform-browser), it imports CommonModule (from @angular/common) on its own, and then I don't need to import BrowserModule AND ALSO CommonModule in my app. The BrowserModule has all the imports it needs on its own so it can work by just importing it without importing any extra stuff manually myself. Or is this a special case?
(I said to many times "imports/ing", hope I made myself clear enough :) )
Ok, let's make a bit of history:
initially Angular 2 was not split into packages, and every simple function was imported, for example, also NgIf. Then it was divided into packages, then the modules were introduced: the _CommonModule_ was considered in the same way as the _core_: it is exported from the _BrowserModule_ so that for a basic app you can only import the _BrowserModule_ : but this is valid only for the root: if you use the lazy loaded modules then you must explicitly import the _CommonModule_. If you look at the code, you can also see an error handling for this: https://github.com/angular/angular/blob/master/packages/platform-browser/src/browser.ts#L95
As said, it is not a best practice for an external library:
@damienbod I think we should explicitly add in the docs that this library depends on Angular _HttpModule_ (tomorrow _HttpClient_) and _RouterModule_
@robisim74 Will do it, thanks
@CesarD @robisim74 @FabianGosebrink
I think we should also remian with this module so that we support all Angular versions from 4 onwards
When Angular goes 5, we could consider this again.
@robisim74 Thanks a lot for the thorough explanation! Much appreciated!!
@damienbod perfect, thanks to Roberto's explanation I see why the breaking change. Should I leave the state of this issue up to you or prefer me to close it in the meantime?
Most helpful comment
Ok, let's make a bit of history:
initially Angular 2 was not split into packages, and every simple function was imported, for example, also
NgIf. Then it was divided into packages, then the modules were introduced: the _CommonModule_ was considered in the same way as the _core_: it is exported from the _BrowserModule_ so that for a basic app you can only import the _BrowserModule_ : but this is valid only for the root: if you use the lazy loaded modules then you must explicitly import the _CommonModule_. If you look at the code, you can also see an error handling for this: https://github.com/angular/angular/blob/master/packages/platform-browser/src/browser.ts#L95As said, it is not a best practice for an external library:
@damienbod I think we should explicitly add in the docs that this library depends on Angular _HttpModule_ (tomorrow _HttpClient_) and _RouterModule_