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 the NgModule when using --module.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 37 (10 by maintainers)

Commits related to this issue

Most upvoted comments

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 the app or common 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 do providedIn: 'UsersModule' without the explicit import of UsersModule which trips the circular dependency detection?

You might wanna strip the following from the docs, to not give people any good ideas:

import { Injectable } from '@angular/core';
import { HeroModule } from './hero.module';
import { HEROES }     from './mock-heroes';

@Injectable({
  // we declare that this service should be created
  // by any injector that includes HeroModule.

  providedIn: HeroModule,
})
export class HeroService {
  getHeroes() { return HEROES; }
}

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)

  1. it’s better,
  2. it solves the “duplicated services instance” problem on shared lazy loaded modules
  3. It’s easier to learn and to use

service–[provides itself to]–>module–[declares]–>component–[depends on]–>service is going to be a very common pattern because in the majority of cases it makes just sense that modules and their components consume their own 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

  1. remove the --module option entirely, or
  2. show a message that it’s probably not what you want when you use that option.

People 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:

  1. Don’t use 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 UserModule
  2. Mostly always use ‘root’. The service is will be available application wide. If the service is only used within a lazy loaded module it will be lazy loaded with that module, if it is never used it will be tree shaked.
  3. If you want to limit the scope of a service remove providedIn and provide it within your lazy loaded module the old fashioned way (preferred) OR provide it within a component
  4. Note: Don’t provide services by a shared module
  5. Note: If you provide your service within a sub-module, it will by available application wide unless the sub-module is lazy loaded.

Can 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: []

@NgModule({ providers: [SomeService] })
export class SomeModule {}
  • old style

  • service will be global singleton if SomeModule is imported eagerly

  • service will be instantiated per lazy module if SomeModule is loaded lazily

  • injecting SomeService in eager part of application will result in error if SomeModule is loaded lazily

  • DISADVANTAGE - mixing of the old providers: [] and the new providedIn syntaxes

Use providedIn

@Injectable({ providedIn: SomeSubModule })
export class SomeService {}

@NgModule({ imports: [SomeSubModule]})
export class SomeModule
  • new providedIn syntax

  • service will be global singleton if SomeModule is imported eagerly

  • service will be instantiated per lazy module if SomeModule is loaded lazily

  • injecting SomeService in eager part of application will result in error if SomeModule is loaded lazily

  • DISADVANTAGE - extra module to prevent circular deps

For me it feels like this feature, while nice in theory, isn’t thought through…

People 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.

Well you totally busted me right now.

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 do providedIn: ‘UsersModule’ without the explicit import of UsersModule which trips the circular dependency detection?

+1 for this idea.

@NgModule({ selector: 'heroes' })
export class HeroModule {}

@Injectable({ providedIn: 'heroes' })
export class HeroService {}

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 the declarations and exports arrays, so couldn’t this have been done for providers as well - before they’re added (or not) to the injector - instead of introducing a new syntax…?

You might wanna strip the following from the docs, to not give people any good ideas:

import { Injectable } from '@angular/core';
import { HeroModule } from './hero.module';
import { HEROES }     from './mock-heroes';

@Injectable({
  // we declare that this service should be created
  // by any injector that includes HeroModule.

  providedIn: HeroModule,
})
export class HeroService {
  getHeroes() { return HEROES; }
}

Source: https://angular.io/guide/dependency-injection#injectable-providers

Or keep it, and put a very big fat warning box next to it.

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…

providedIn: 'root' is the 99% of cases.

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:

Error: Uncaught (in promise): Error: StaticInjectorError(AppModule)[HttpClient]: 
  StaticInjectorError(Platform: core)[HttpClient]: 
    NullInjectorError: No provider for HttpClient!

My LazyLoadedModule has an import on HttpClientModule, but AppModule 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 NgModule providers, 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.