Confusing DI behavior for ChangeDetectorRef

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

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior
I'm not sure this is a bug. If it is, please fix. If not, please clarify the documentation OR (preferrably imo) change the behavior.

This works (no exception thrown)

@Injectable()
class MySvc {
  constructor(cdr:ChangeDetectorRef) {}
}

@Component({
  ...
  providers: [MySvc]
}) 
class MyComponent {
  constructor() {m:MySvc}
}

...whereas this doesn't (it throws a NoProviderError)

// app.module.ts
@NgModule({
  ...,
  providers: [{
    provide: MyService,
    deps: [ChangeDetectorRef],
    useFactory: cdr => new MyService(cdr)
  }]
})

// my-component.ts
@Component({...}) 
class MyComponent {
  constructor() {m:MySvc}
}

Furthermore, this results in an error (which also seems surprising)

@Component({...})
class MyComponent {
  constructor(inj:Injector, cdRef:ChangeDetectorRef) {
    inj.get(ChangeDetectorRef);
  }
}

Expected behavior
I would expect all these things to work. But it seems ChangeDetectorRef (and other types close to view creation?) are special. They aren't injected normally and sometimes work and sometimes not, with very surprising results.

So wish list is, in priority order:

Make changes so that these examples actually work, by always providing the ChangeDetectorRef (and potentially other injectables) to the component injector.
Document which classes are special and why

What is the motivation / use case for changing the behavior?
I, and others, find it strange and run into confusing behavior.

Angular version:
2.4.2

@Injectable()
class MySvc {
  constructor(cdr:ChangeDetectorRef) {}
}

@Component({
  ...
  providers: [MySvc]
}) 
class MyComponent {
  constructor() {m:MySvc}
}

ok, component's providers have access to component's cdr.

@NgModule({
  ...,
  providers: [{
    provide: MyService,
    deps: [ChangeDetectorRef],
    useFactory: cdr => new MyService(cdr)
  }]
})

ok, module doesn't know anything about cdr.

@Component({...})
class MyComponent {
  constructor(inj:Injector, cdRef:ChangeDetectorRef) {
    inj.get(ChangeDetectorRef);
  }
}

~~this is looks like a bug~~ link as you can see, cdr is a special builtin type, and angular uses a ref field instead of injector to resolve it. So it works as intended.

@DzmitryShylovich

Thanks for the reply @DzmitryShylovich

However, linking to the code to say that this is how it works kind of misses my point. Which was, to reiterate:

I don't think this is a good behavior and I think you should change it
If you don't agree, then I think you should at least improve the documentation to explain what types work this way and why

As for the use case:
Sure, we could always add it to the providers array for components, but it's a little terse, and above all, confusing. Having access to a component's ChangeDetectorRef for providers enables e.g. the following for us:

setting up a generalized way of consuming our observables and mark components for checking
(related) having access to a component's ElementRef enables debug visualizations in the dom.

Is there a reason you chose not to add the ChangeDetectorRefto the component injector? My thinking is you reduce complexity by having fewer exceptions to the rules.

Having access to a component's ChangeDetectorRef for providers

every component has its own element ref, cdr, view container

every component has its own element ref, cdr, view container

um...yeah of course, I don't think that's been questioned

@staeke

@staeke you need to take into account that angular has an hierarchical injector, therefore there are some types that can only be provided at specific levels of that hierarchy, like ChangeDetectorRef, ElementRef, etc.
That's why it works when you define it in the providers array, you are registering at the same level of the component and, therefore, can ask for dependencies of that level.

It works this way because these injectables are tightly coupled to a specific element/component, that's generally (most likely always) the case when you encounter a Ref, it's a local instance.

On your second example you are registering it at the module level. That's a problem since these specific types of injectables are directly associated with an element/view/component, as such, the module 'scope' does not provide these injectables because they are defined at a lower level of the tree, and injector lookups only propagate up, not down.

On your third example I suspect (did no actual research) the Injector refers to the root injector, which, obeys the same rules as the module one, it does not know about these low level and coupled Injectables.

If that's a problem or not, I leave it for the team and the community to decide.
Personally I think it makes sense to keep it hierarchical and unidirectional

As a side note, sharing refs around is most likely a bad idea, since you are delegating control to 'outsiders'
You can always create your repository of ChangeDetectorRef and ElementRef and register your components at construction time but I would strongly, very strongly recommend against it, a pub/sub like implementation would probably be a better choice.

@fabioaparicio

@fabioaparicio Thanks for taking the time to write a thorough reply.

First, let me clarify something I actually missed before: If you inject a provider at a component level that, and that provider injects a ChangeDetectorRef it actually gets the component's parent view's cd ref (my first example).

Then, again, my question is not so much about "why does it work this way" - I get that. It's about whether this is a good idea, but mainly lack of documentation.

Now some comments on what you wrote:

Personally I think it makes sense to keep it hierarchical and unidirectional

I don't question hierarchical injectors. Neither unidirectional data flows

On your third example I suspect (did no actual research) the Injector refers to the root injector, which, obeys the same rules as the module one, it does not know about these low level and coupled Injectables.

The third example doesn't fail because of that. You get the component's injector, but ChangeDetectorRef along with ElementRef and ViewContainerRef are special in that they are not registered with the container. Now, after digging more I realize that this is because there might be many objects of these kind as there is a parent/children relationship around each component's views. It's better explained by looking at the ComponentRef created. This is roughly what it looks like

ComponentRef
  elementRef (E1) (points to <my-component>)
  changeDetectorRef (CDR1) (very unclear what this does)
  injector
  compView0:
     elementRef (E2) (also points to <my-component>)
     changeDetectorRef (CDR2) (NOT equal to CDR1)

This is quite contrary to how, at least I, assumed things to be. Given this setup it's hard to register e.g. a cd ref with the injector, as there is potentially lots of them. And a couple of questions come out of it:

Why isn't the MyComponent designed as the root of this structure? It seems that would make more sense. I'm guessing performance reasons so that dynamically created views inside the component that use OnPush shouldn't cause new OnCheck rounds in the "parent" component. Which seems unintuitive to me. Again - why? Documentation please.
What good is CDR1? Fooled me by thinking i had CDR2, but nothing happended on CDR1.markForCheck(). Again - documentation please.
Haven't tried - but this example should probably also fail then (and probably surprising people)

compRef = createComponent(...)
...
compRef.changeDetectorRef.markForCheck() // Wrong changeDetectorRef

As a side note, sharing refs around is most likely a bad idea, since you are delegating control to 'outsiders'
You can always create your repository of ChangeDetectorRef and ElementRef and register your components at construction time but I would strongly, very strongly recommend against it, a pub/sub like implementation would probably be a better choice.

This is an interesting feedback. First, registering works, but it's a little terse. Moving on...regarding whether this is good/bad I think we need to consider use cases more. I fail to see how pub/sub is an alternative to my use case. I previously mentioned setting up a generalized way of consuming our observables and mark components for checking. Something like the async pipe in typescript code. Let's look at a code example to be concrete

@Component({....changeDetection:ChangeDetectionStrategy.OnPush..})
class MyComponent {
  sub: AnonymousSubscription;

  constructor(data:DataSvc, cdr:ChangeDetectorRef) {
     this.sub = data.someObservable.subscribe(val => {
        DoSomething();
        cdr.markForCheck();
     })
  }
}

This would be much less error prone and error prone as

subscribeNgComp(this, data.someObservable, val => DoSomething())

(which could be refactored to even more compact annotations, Observable extensions etc.)

Without a component -> CDR lookup, you'd have to write something like this and monkey patch ngOnDestroy)

constructor(cdr:ChangeDectectorRef)
subscribeNgComp(this, cdr, data.someObservable, val => DoSomething())

Even more preferrable would be not to have to monkey patch ngOnDestroy. Then we'd have to get hold of either the ComponentRef or the ViewRef which seems close to impossible. The Router broadcasts ComponentRef's for its created components, but that's about it. It also seems close to impossible to override the default ComponentFactory in order to keep a registry. Don't know why we can't inject ViewRef - again: documentation would be nice.

Could you help me summarize the issues you reported here?

ChangeDetectorRef is not injectable in regular providers (btw as ElementRef and ViewContainerRef and TemplateRef)
The ChangeDetectorRef retrieved via injector.get(ChangeDetectorRef) is not the one of the component but rather the one of the parent view

Regarding getting the wrong ChangeDetectorRef also see #12816

ChangeDetectorRef can be requested from any node Injector instance with Angular Ivy since v9, so closing as resolved.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

Navigation Menu

Repository navigation

Confusing DI behavior for ChangeDetectorRef #14656

5 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Confusing DI behavior for ChangeDetectorRef #14656

Description

Activity

DzmitryShylovich commented on Feb 22, 2017

staeke commented on Feb 23, 2017

DzmitryShylovich commented on Feb 23, 2017

staeke commented on Feb 23, 2017

flga commented on Mar 10, 2017

staeke commented on Mar 11, 2017

tbosch commented on Apr 11, 2017

tbosch commented on Apr 11, 2017

5 remaining items

JoostK commented on May 24, 2022

angular-automatic-lock-bot commented on Jun 24, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions