Nested views injection fix #1244
Conversation
|
Discussed with @rkirov and we decided that we need a better fix. Helped him create a better test, he is working on the proper fix. |
| final Node _node; | ||
| final NodeAttrs _nodeAttrs; | ||
| final Animate _animate; | ||
| final EventHandler _eventHandler; | ||
| final EventHandler eventHandler; |
There was a problem hiding this comment.
Lets keep eventHandler private Use this trick: https://github.com/angular/angular.dart/blob/master/lib/core/annotation_src.dart#L16
Replace DefaultDirectiveInjector with DirectiveInjector (with parent = null). Application Injector reference is passed through view creation and passed into new Directive Injector (instead of using parentInjector.appInjector, which is wrong). New public method on dependency injector - parentGet. Allowing to get through the usual chain but skipping itself (used for NgControl). Remove EventListener from View. Closes #1244
Replace DefaultDirectiveInjector with DirectiveInjector (with parent = null). Application Injector reference is passed through view creation and passed into new Directive Injector (instead of using parentInjector.appInjector, which is wrong). New public method on dependency injector - parentGet. Allowing to get through the usual chain but skipping itself (used for NgControl). Remove EventListener from View. Closes #1244
| @@ -64,7 +64,6 @@ part 'ng_model_options.dart'; | |||
| */ | |||
| class DirectiveModule extends Module { | |||
| DirectiveModule() { | |||
| bind(DirectiveInjector, toImplementation: DefaultDirectiveInjector); | |||
There was a problem hiding this comment.
You need to bind something to DirectiveInjector. Currently, this fails with an error that there is no provider for DirectiveInjector in an application that I'm upgrading to support this change (after replacing all .parent.get with .parentGet.)
|
This commit provides no default implementation for DirectiveInjector causing an exception. I'm reverting it from master and reopening this PR. |
| get parent => this._parent; | ||
|
|
||
| // Same behavior as get but skips the current injector. | ||
| Object parentGet(Type type) => parentGetByKey(new Key(type)); |
There was a problem hiding this comment.
what about getFromParent(ByKey) ?
There was a problem hiding this comment.
Sounds good. In fact, injector.parent seems to be widely used, so I will introduce these new APIs first in a separate PR, so that the apps can be switched away from injector.parent landing this significantly bigger PR.
|
rebased. I handled all the clients breakage by moving them to the new APIs (getFromParent), so this should be ready to submit. |
|
@rkirov yup, I had a few PRs touching the same code. From what I can see there will be quite a few conflicts, but nothing incompatible. |
rkirov
force-pushed
the
nested-view-fix
branch
from
ebf576b to
5f7851f
Compare
Breaking change: Regular (application) injectors cannot construct DirectiveInjectors (DI). Only compilers create DI as part of view creation process. Deprecation: directive injector parent is now private. New public method on dependency injector - parentGet, which allows to get through the usual chain but skipping itself. Component Injectors now break the resolution chain (except when called directly.) TestBed does not need DI in its constructor. Internal changes: - Application Injector reference is passed through view creation and passed into new Directive Injector (instead of using parentInjector.appInjector, which is wrong when used with ng-view). - Unwind recursion from the directive injector. - Remove EventListener from View. - Replace DefaultDirectiveInjector with DirectiveInjector (with parent = null). - Component visibility handled outside the visibility enum. - Removed Shadowless and ShadowDirectiveInjector subclasses. Closes dart-archive#1111
rkirov
force-pushed
the
nested-view-fix
branch
from
5f7851f to
3607d62
Compare
|
Merged last week as 600113a |
forceNewDirectivesAndFormatters is now respecting inheritance for directive and application injectors.