Skip to content

Commit d800d2f

Browse files
rkirovrkirov
committed
fix(injectors): sync injector tree with dom element tree.
Changes adds createGrowableSize method to allow for growable lists with fixed start. Closes: angular#2498
1 parent 24646e7 commit d800d2f

File tree

7 files changed

+64
-26
lines changed

7 files changed

+64
-26
lines changed

‎modules/angular2/src/core/compiler/element_injector.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,8 @@ export class ElementInjector extends TreeNode<ElementInjector> {
10271027
if (queryRef.query.descendants == false) {
10281028
if (this == queryRef.originator) {
10291029
this._addQueryToTreeSelfAndRecurse(queryRef);
1030-
} else if (this.parent == queryRef.originator && this._proto.distanceToParent == 1) {
1030+
// TODO(rado): add check for distance to parent = 1 when issue #2707 is fixed.
1031+
} else if (this.parent == queryRef.originator) {
10311032
this._assignQueryRef(queryRef);
10321033
}
10331034
} else {

‎modules/angular2/src/core/compiler/view_manager_utils.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ export class AppViewManagerUtils {
8686
this._hydrateView(hostView, injector, null, new Object(), null);
8787
}
8888

89+
// Misnomer: this method is attaching next to the view container.
8990
attachViewInContainer(parentView: viewModule.AppView, boundElementIndex: number,
9091
contextView: viewModule.AppView, contextBoundElementIndex: number,
9192
atIndex: number, view: viewModule.AppView) {
@@ -104,7 +105,11 @@ export class AppViewManagerUtils {
104105
}
105106
var elementInjector = contextView.elementInjectors[contextBoundElementIndex];
106107
for (var i = view.rootElementInjectors.length - 1; i >= 0; i--) {
107-
view.rootElementInjectors[i].linkAfter(elementInjector, sibling);
108+
if (isPresent(elementInjector.parent)) {
109+
view.rootElementInjectors[i].linkAfter(elementInjector.parent, sibling);
110+
} else {
111+
contextView.rootElementInjectors.push(view.rootElementInjectors[i]);
112+
}
108113
}
109114
}
110115

@@ -115,7 +120,13 @@ export class AppViewManagerUtils {
115120
view.changeDetector.remove();
116121
ListWrapper.removeAt(viewContainer.views, atIndex);
117122
for (var i = 0; i < view.rootElementInjectors.length; ++i) {
118-
view.rootElementInjectors[i].unlink();
123+
var inj = view.rootElementInjectors[i];
124+
if (isPresent(inj.parent)) {
125+
inj.unlink();
126+
} else {
127+
var removeIdx = ListWrapper.indexOf(parentView.rootElementInjectors, inj);
128+
ListWrapper.removeAt(parentView.rootElementInjectors, removeIdx);
129+
}
119130
}
120131
}
121132

‎modules/angular2/src/directives/ng_for.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export class NgFor {
107107
return movedTuples;
108108
}
109109

110-
static bulkInsert(tuples, viewContainer, protoViewRef) {
110+
static bulkInsert(tuples, viewContainer: ViewContainerRef, protoViewRef: ProtoViewRef) {
111111
tuples.sort((a, b) => a.record.currentIndex - b.record.currentIndex);
112112
for (var i = 0; i < tuples.length; i++) {
113113
var tuple = tuples[i];

‎modules/angular2/src/facade/collection.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ class StringMapWrapper {
9393
class ListWrapper {
9494
static List clone(Iterable l) => new List.from(l);
9595
static List createFixedSize(int size) => new List(size);
96+
static List createGrowableSize(int size) =>
97+
new List.generate(size, (_) => null, growable: true);
9698
static get(List m, int k) => m[k];
9799
static void set(List m, int k, v) {
98100
m[k] = v;

‎modules/angular2/src/facade/collection.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,10 @@ export class StringMapWrapper {
140140
}
141141

142142
export class ListWrapper {
143+
// JS has no way to express a staticly fixed size list, but dart does so we
144+
// keep both methods.
143145
static createFixedSize(size): List<any> { return new List(size); }
146+
static createGrowableSize(size): List<any> { return new List(size); }
144147
static get(m, k) { return m[k]; }
145148
static set(m, k, v) { m[k] = v; }
146149
static clone(array: List<any>) { return array.slice(0); }

‎modules/angular2/test/core/compiler/query_integration_spec.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,11 @@ export function main() {
7676
});
7777
}));
7878

79-
// TODO(rado): The test below should be using descendants: false,
80-
// but due to a bug with how injectors are hooked up query considers the
81-
// directives to be distances 2 instead of direct children.
8279
it('should reflect dynamically inserted directives',
8380
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
8481
var template =
8582
'<div text="1"></div>' +
86-
'<needs-query-desc text="2"><div *ng-if="shouldShow" [text]="\'3\'"></div></needs-query-desc>' +
83+
'<needs-query text="2"><div *ng-if="shouldShow" [text]="\'3\'"></div></needs-query>' +
8784
'<div text="4"></div>';
8885

8986
tb.createView(MyComp, {html: template})
@@ -104,7 +101,7 @@ export function main() {
104101
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
105102
var template =
106103
'<div text="1"></div>' +
107-
'<needs-query-desc text="2"><div *ng-for="var i of list" [text]="i"></div></needs-query-desc>' +
104+
'<needs-query text="2"><div *ng-for="var i of list" [text]="i"></div></needs-query>' +
108105
'<div text="4"></div>';
109106

110107
tb.createView(MyComp, {html: template})
@@ -126,10 +123,10 @@ export function main() {
126123
describe("onChange", () => {
127124
it('should notify query on change',
128125
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
129-
var template = '<needs-query-desc #q>' +
126+
var template = '<needs-query #q>' +
130127
'<div text="1"></div>' +
131128
'<div *ng-if="shouldShow" text="2"></div>' +
132-
'</needs-query-desc>';
129+
'</needs-query>';
133130

134131
tb.createView(MyComp, {html: template})
135132
.then((view) => {

‎modules/angular2/test/core/compiler/view_manager_utils_spec.ts‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ import {ElementBinder} from 'angular2/src/core/compiler/element_binder';
2727
import {
2828
DirectiveBinding,
2929
ElementInjector,
30-
PreBuiltObjects
30+
PreBuiltObjects,
31+
ProtoElementInjector
3132
} from 'angular2/src/core/compiler/element_injector';
3233
import {DirectiveResolver} from 'angular2/src/core/compiler/directive_resolver';
3334
import {Component} from 'angular2/annotations';
@@ -66,10 +67,12 @@ export function main() {
6667
return res;
6768
}
6869

69-
function createElementInjector() {
70+
function createElementInjector(parent = null) {
7071
var host = new SpyElementInjector();
7172
var appInjector = new SpyInjector();
72-
return SpyObject.stub(new SpyElementInjector(),
73+
var elementInjector =
74+
isPresent(parent) ? new SpyElementInjectorWithParent(parent) : new SpyElementInjector();
75+
return SpyObject.stub(elementInjector,
7376
{
7477
'isExportingComponent': false,
7578
'isExportingElement': false,
@@ -82,15 +85,19 @@ export function main() {
8285
{});
8386
}
8487

85-
function createView(pv = null) {
88+
function createView(pv = null, nestedInjectors = false) {
8689
if (isBlank(pv)) {
8790
pv = createProtoView();
8891
}
8992
var view = new AppView(null, pv, new Map());
90-
var elementInjectors = ListWrapper.createFixedSize(pv.elementBinders.length);
93+
var elementInjectors = ListWrapper.createGrowableSize(pv.elementBinders.length);
9194
var preBuiltObjects = ListWrapper.createFixedSize(pv.elementBinders.length);
9295
for (var i = 0; i < pv.elementBinders.length; i++) {
93-
elementInjectors[i] = createElementInjector();
96+
if (nestedInjectors && i > 0) {
97+
elementInjectors[i] = createElementInjector(elementInjectors[i - 1]);
98+
} else {
99+
elementInjectors[i] = createElementInjector();
100+
}
94101
preBuiltObjects[i] = new SpyPreBuiltObjects();
95102
}
96103
view.init(<any>new SpyChangeDetector(), elementInjectors, elementInjectors, preBuiltObjects,
@@ -118,10 +125,9 @@ export function main() {
118125
var spyCd = <any>componentView.changeDetector;
119126
spyCd.spy('hydrate').andCallFake(log.fn('hydrateCD'));
120127

121-
utils.hydrateComponentView(hostView, 0)
128+
utils.hydrateComponentView(hostView, 0);
122129

123-
expect(log.result())
124-
.toEqual('hydrate; hydrateCD');
130+
expect(log.result()).toEqual('hydrate; hydrateCD');
125131
});
126132

127133
});
@@ -187,25 +193,32 @@ export function main() {
187193
describe('attachViewInContainer', () => {
188194
var parentView, contextView, childView;
189195

190-
function createViews() {
196+
function createViews(numInj = 1) {
191197
var parentPv = createProtoView([createEmptyElBinder()]);
192198
parentView = createView(parentPv);
193199

194-
var contextPv = createProtoView([createEmptyElBinder()]);
195-
contextView = createView(contextPv);
200+
var binders = [];
201+
for (var i = 0; i < numInj; i++) binders.push(createEmptyElBinder());
202+
var contextPv = createProtoView(binders);
203+
contextView = createView(contextPv, true);
196204

197205
var childPv = createProtoView([createEmptyElBinder()]);
198206
childView = createView(childPv);
199207
}
200208

209+
it('should link the views rootElementInjectors at the given context', () => {
210+
createViews();
211+
utils.attachViewInContainer(parentView, 0, contextView, 0, 0, childView);
212+
expect(contextView.elementInjectors.length).toEqual(2);
213+
});
214+
201215
it('should link the views rootElementInjectors after the elementInjector at the given context',
202216
() => {
203-
createViews();
204-
utils.attachViewInContainer(parentView, 0, contextView, 0, 0, childView);
217+
createViews(2);
218+
utils.attachViewInContainer(parentView, 0, contextView, 1, 0, childView);
205219
expect(childView.rootElementInjectors[0].spy('linkAfter'))
206220
.toHaveBeenCalledWith(contextView.elementInjectors[0], null);
207221
});
208-
209222
});
210223

211224
describe('hydrateViewInContainer', () => {
@@ -279,6 +292,17 @@ class SpyElementInjector extends SpyObject {
279292
noSuchMethod(m) { return super.noSuchMethod(m) }
280293
}
281294

295+
@proxy
296+
@IMPLEMENTS(ElementInjector)
297+
class SpyElementInjectorWithParent extends SpyObject {
298+
parent: ElementInjector;
299+
constructor(parent) {
300+
super(ElementInjector);
301+
this.parent = parent;
302+
}
303+
noSuchMethod(m) { return super.noSuchMethod(m) }
304+
}
305+
282306
@proxy
283307
@IMPLEMENTS(PreBuiltObjects)
284308
class SpyPreBuiltObjects extends SpyObject {

0 commit comments

Comments
 (0)