angular-cli: Circular dependency error with service generated with --module in 6.0.0-rc.0
Versions
Angular CLI: 6.0.0-rc.0
Node: 8.11.0
OS: darwin x64
Angular: 6.0.0-rc.1
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router
@angular/cli: 6.0.0-rc.0
@angular-devkit/architect: 0.0.10
@angular-devkit/build-angular: 0.0.10
@angular-devkit/build-optimizer: 0.4.9
@angular-devkit/core: 0.4.9
@angular-devkit/schematics: 0.5.0
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 6.0.0-beta.9
@schematics/angular: 0.5.0
@schematics/update: 0.5.0
typescript: 2.7.2
webpack: 4.1.0
Repro steps
- ng new hello
- ng g module world --module app
- ng g service world/data --module world
- ng g component world/page
- inject the service in the component constructor
Observed behavior
WARNING in Circular dependency detected:
src/app/world/data.service.ts -> src/app/world/world.module.ts -> src/app/world/page/page.component.ts -> src/app/world/data.service.ts
Desired behavior
CLI v6 ng g service
command now provides the service in a tree-shakeable way, using the providedIn
option of Injectable
decorator.
Without the --module
option, it uses providedIn: 'root'
, which seems OK.
But with the --module
option, CLI v1 was providing the service directly inside the NgModule
. Now in CLI v6, the module is configured via providedIn
. But then the service is not usable in components, which is a serious problem.
I see 2 options :
- either it is an Angular issue, which should be resolved ;
- either it is a known limitation of
providedIn
, then the CLI should keep its old behavior of providing the service directly in theNgModule
when using--module
.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Comments: 37 (10 by maintainers)
Links to this issue
Commits related to this issue
- fix(@schematics/angular): Remove module option from service schematic fixes angular/angular-cli#10170 — committed to Brocco/devkit by Brocco 6 years ago
- fix(@schematics/angular): Remove module option from service schematic fixes angular/angular-cli#10170 — committed to Brocco/devkit by Brocco 6 years ago
- fix(@schematics/angular): Remove module option from service schematic fixes angular/angular-cli#10170 — committed to angular/devkit by Brocco 6 years ago
- fix(@schematics/angular): Remove module option from service schematic fixes angular/angular-cli#10170 — committed to angular/angular-cli by Brocco 6 years ago
I’m experiencing this problem as well, but am unconvinced that
providedIn: 'root'
is a good longterm solution for every service. For instance, I was under the impression that lazy-loaded modules bundled their provided services with them, meaning this new method would swell theapp
orcommon
bundle considerably. If that’s not the case, then I guess I’m on board, but I’d be surprised to learn that.Since
'root'
is a string Token to look up the root module, why not allow us to declare module selectors in the same vein? That way you could doprovidedIn: 'UsersModule'
without the explicit import ofUsersModule
which trips the circular dependency detection?You might wanna strip the following from the docs, to not give people any good ideas:
Source: https://angular.io/guide/dependency-injection#injectable-providers
Or keep it, and put a very big fat warning box next to it.
After playing with this for the last hour, I have come to the following conclusion that for services used across the application, then it is only useful to have
providedIn: 'root'
. Do not bother putting these into the providers array of the AppModule.However, if you want to provide a service in any feature module (not the root), then you are better off using the providers array in the feature module’s decorators, otherwise you will be plagued with circular dependency warnings.
From reading lots of things about the topic, what I understand is the following:
Like @hansl wrote: "Always use
providedIn: 'root'
Like @swimmadude66 wrote: it will still be bundled in the right bundle (so they the build does the job for you if it’s only used in the lazy loaded bundle)
And just like in this issue 23715 - If you really want to limit the scope, you can do it with a separate module for services which gets loaded by the lazy loaded module.
The only thing missing is to update the docs and I think it’ll be better to remove the section that shows you can use
providedIn
with specific modules (or just hide it in a “super rare cases” section).I think that will reduce the confusion and will promote the better practice. (FYI @jenniferfell )
CONCLUSION:
Always use the
providedIn: 'root'
, (for both eager + lazy loaded modules’ services)Please stop teaching or telling pepole to use this pattern. With tree shakeable providers, you now have zero reasons to provide a service from a module (there is a rare case on some lazy loaded services with a component using the service from a separate lazy module). The
--module
flag is there as a courtesy, but there’s a reason it’s not the default; it shouldn’t be used.providedIn: 'root'
is the 99% of cases.What will likely happen is; we will either
--module
option entirely, orPeople using
--module
without knowing what they’re doing need to see the circular dependency error and tell themselves “that’s odd”, and search and find that they shouldn’t use it. Hopefully we’ll stop using this pattern entirely in courses / tutorials / examples.Has everyone read the updated documentation yet? Also there is the FAQ providing tons of informations how it works (probably better than the article on softwarearchitect.io ?)
If i get this right:
providedIn: UserModule
if you develop a application. It is only needed for developing libraries: When someone is using your library he cant provide your service if he didn’t import UserModuleprovidedIn
and provide it within your lazy loaded module the old fashioned way (preferred) OR provide it within a componentCan someone confirm?
Whoa Nellie!
I have a large number of services that are scoped to modules (lazy modules) or even to components within those (lazy modules).
This pattern has worked GREAT for a very long time.
I understand the desire to make LIBRARIES treeshakable. A library often includes tons of code that many consumers will not need or want.
However, APPLICATIONS typically contain very few treeshakable services. If I write a service for my app, it’s because I expect the app to need it. I don’t write app services that someone might NOT need.
Therefore, treeshaking is of very little benefit to application code.
If you think I’m wrong, show me (us) please.
FWIW, I know we can use the new syntax to declare that a service is provided in a non-root module (and I think it works in a component too). That’s grand, but do I need it? In an application, I don’t think that I do.
Moreover, this style is a mixed blessing. On the plus side, it’s easy to tell by reading the service where it is provided and to ensure that it is provided there. On the negative side, I’ve pinned the service to that particular usage. I can’t safely provide it at another level because I’m at risk of service shadowing (a serious problem for a stateful service).
It feels premature to make such bold statements about a feature that none of us could have used for more than a few months.
As an update to my last post: it does appear that even when
providedIn: 'root'
webpack will bundle the service with the correct lazy module bundle, so no concern there.If it should be
providedIn: 'root'
in virtually all cases then having to specify it only serve the purpose of confusing people.@MickL I see that, so then it is a true statement to say that these two are equivalent? @hansl
Use providers: []
old style
service will be global singleton if
SomeModule
is imported eagerlyservice will be instantiated per lazy module if
SomeModule
is loaded lazilyinjecting
SomeService
in eager part of application will result in error ifSomeModule
is loaded lazilyDISADVANTAGE - mixing of the old
providers: []
and the newprovidedIn
syntaxesUse providedIn
new
providedIn
syntaxservice will be global singleton if
SomeModule
is imported eagerlyservice will be instantiated per lazy module if
SomeModule
is loaded lazilyinjecting
SomeService
in eager part of application will result in error ifSomeModule
is loaded lazilyDISADVANTAGE - extra module to prevent circular deps
For me it feels like this feature, while nice in theory, isn’t thought through…
Well you totally busted me right now.
+1 for this idea.
I think that’s a nice way to do it. This should be possible, right?
Please correct me if I’m missing something, but it seems to me that adding the
providedIn: 'root'
syntax breaks the link between the service and the module it nominally lives in?The service doesn’t know about the module, and the module doesn’t know about the service anymore, so the service is no longer really “in” the module. Instead, the service will just get bundled with whatever references it?
The result of this seems to be that we can no longer guarantee that the service’s dependencies (as defined by the module it “lives in”) will be loaded when the service is injected, hence - I think - the problem described by @luchsamapparat?
It seems to me this could be fixed using the
providedIn: 'UserModule'
idea, suggested by @swimmadude66 (which allows the module to be specified, without causing a circular reference).Alternatively though, wouldn’t it have been simpler not to have introduced the
providedIn
syntax in the first place? Tree shaking of components is already performed, even when they are referenced in thedeclarations
andexports
arrays, so couldn’t this have been done forproviders
as well - before they’re added (or not) to the injector - instead of introducing a new syntax…?This has just happend to me using this page:
https://angular.io/guide/providers#providing-a-service
Please guys could you remove it from de docs? Or add something that warning the people. I saw the problem after implement it.
I stumbled upon the same issue @fxck described when using services in ngrx effects:
service--[provides itself to]-->module--[declares]-->effect--[depends on]--service
In my case, both service and effect are within a lazy-loaded module. I used
providedIn: LazyLoadedModule
on my service and got the circular dependency warning.Then I found this thread, telling me…
Alright. So I changed my service to be provided by
root
. But now, I get an error when my lazy-loaded module is loaded during runtime:My
LazyLoadedModule
has an import onHttpClientModule
, butAppModule
doesn’t. Why should it? AppModule in my case doesn’t do anything (for the most part) besides being a shell for lazy-loaded modules. Why should I declare a dependency here that is required by a service in a lazy-loaded module?I’m lost on what is the best practice here… 🤔
Angular contributor says make all your service instances Root scoped Singletons? Regardless of the programming language or framework this can’t be a serious mindset. If I wanted to do silly things with code I would switch to React and become a JSX fan.
Being able to control the scope and lifetime of an injected dependency instance is very important, I hope the Angular Team is not planning on taking this capability away for tree shaking improvements. What you end up with is a small file bundle that quickly becomes bloated in memory at run time.
I don’t use ‘providedIn’ because I like being able to control scope and lifetime of my service instances, and when a developer has this mindset then there is no need for tree shaking since I intend for my services to be used else I would not go to the trouble of providing them only where they are needed.
Since the CLI defaults to providedIn when generating a service class I see teams of Angular newbies using it everywhere in their apps for services that are written to work only with a single component instance. I would like to see the providedIn technique be Opt In and remove the {providedIn: ‘root’} snippet from CLI generated services.
@cedvdb I’m with you, if that was the only way to define providers. But I guess they made us configure it this way not to break existing projects, because the
providedIn
way does change the functionality slightly.@hansl Thanks for the explanation.
I think deleting the
--module
option right now would be the right way to go. Indeed, before v6, we had to provide services in NgModuleproviders
, which was done with the CLI with--module
. So people will first continue to use--module
but suddenly in V6 it will do a new behavior which you say is bad and leads to errors.That’s a good point. I’m asking around if this is by design. It’s worth noting that the circular dependency detection was introduced in the past because there were cases where it broke bundling. Perhaps it is not actually needed anymore.