fix(cdk/dialog): use config injector if provided #25277
Conversation
src/cdk/dialog/dialog.ts
Outdated
| @@ -308,7 +308,7 @@ export class Dialog implements OnDestroy { | |||
| dialogRef: DialogRef<R, C>, | |||
| dialogContainer: BasePortalOutlet, | |||
| ): Injector { | |||
| const userInjector = config && config.viewContainerRef && config.viewContainerRef.injector; | |||
| const userInjector = config.injector ?? config.viewContainerRef?.injector; | |||
There was a problem hiding this comment.
This injector is only used to resolve the layout direction of the dialog (ltr vs rtl). I think that we actually want to use the ViewContainerRef here specifically since its tied to a particular location in the app that might have a different direction than the user-provided injector.
There was a problem hiding this comment.
This injector is also used for providing. Because I can see that, without this change, the providers inside lazy-loaded modules are failing. And with that change, they are successfully provided.
I can understand that you want to use the injector inside ViewContainerRef. But in this case, it has to be a ViewContainerRef built inside this class and not a config one.
Indeed, in my case, I do not construct manually the ViewContainerRef. So the config.viewContainerRef is undefined.
Moreover, the name of this parameter is userInjector (same as line 224). As a consequence, it seemed logic to me that this parameter could be overrided by user and not be kept for an internal purpose.
There was a problem hiding this comment.
Okay, I see what you mean. I think that we should still resolve the Directionality with the ViewContainerRef injector first, but the user injector can take precedence in the Injector.create call below. We'll need a unit test for the change as well.
There was a problem hiding this comment.
Ok. We have replaced the use of config.injector inside Injector.create and added a unit test as requested.
ldulary
force-pushed
the
fix_dialog_injector
branch
from
3a3bdf4 to
4e165f9
Compare
|
This change was merged in with ea4e203. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Cdk Dialog provides a field injector in config object. We would gladly use it to provide an injector issued from our lazy-loaded module.
It is correctly used on line 224 : const userInjector = config.injector ?? config.viewContainerRef?.injector;
But not on line 311.
So I guess it would be better to use the same "user" injector on both lines.