Skip to content

Commit

Permalink
fix(injectors): sync injector tree with dom element tree.
Browse files Browse the repository at this point in the history 
Changes adds createGrowableSize method to allow for growable lists with fixed
start.

Closes: #2498
  • Loading branch information
@rkirov
rkirov committed Jun 24, 2015
1 parent 24646e7 commit d800d2f
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 26 deletions.
3 changes: 2 additions & 1 deletion modules/angular2/src/core/compiler/element_injector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,8 @@ export class ElementInjector extends TreeNode<ElementInjector> {
if (queryRef.query.descendants == false) {
if (this == queryRef.originator) {
this._addQueryToTreeSelfAndRecurse(queryRef);
} else if (this.parent == queryRef.originator && this._proto.distanceToParent == 1) {
// TODO(rado): add check for distance to parent = 1 when issue #2707 is fixed.
} else if (this.parent == queryRef.originator) {
this._assignQueryRef(queryRef);
}
} else {
Expand Down
15 changes: 13 additions & 2 deletions modules/angular2/src/core/compiler/view_manager_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export class AppViewManagerUtils {
this._hydrateView(hostView, injector, null, new Object(), null);
}

// Misnomer: this method is attaching next to the view container.
attachViewInContainer(parentView: viewModule.AppView, boundElementIndex: number,
contextView: viewModule.AppView, contextBoundElementIndex: number,
atIndex: number, view: viewModule.AppView) {
Expand All @@ -104,7 +105,11 @@ export class AppViewManagerUtils {
}
var elementInjector = contextView.elementInjectors[contextBoundElementIndex];
for (var i = view.rootElementInjectors.length - 1; i >= 0; i--) {
view.rootElementInjectors[i].linkAfter(elementInjector, sibling);
if (isPresent(elementInjector.parent)) {
view.rootElementInjectors[i].linkAfter(elementInjector.parent, sibling);
} else {
contextView.rootElementInjectors.push(view.rootElementInjectors[i]);
}
}
}

Expand All @@ -115,7 +120,13 @@ export class AppViewManagerUtils {
view.changeDetector.remove();
ListWrapper.removeAt(viewContainer.views, atIndex);
for (var i = 0; i < view.rootElementInjectors.length; ++i) {
view.rootElementInjectors[i].unlink();
var inj = view.rootElementInjectors[i];
if (isPresent(inj.parent)) {
inj.unlink();
} else {
var removeIdx = ListWrapper.indexOf(parentView.rootElementInjectors, inj);
ListWrapper.removeAt(parentView.rootElementInjectors, removeIdx);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/angular2/src/directives/ng_for.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export class NgFor {
return movedTuples;
}

static bulkInsert(tuples, viewContainer, protoViewRef) {
static bulkInsert(tuples, viewContainer: ViewContainerRef, protoViewRef: ProtoViewRef) {
tuples.sort((a, b) => a.record.currentIndex - b.record.currentIndex);
for (var i = 0; i < tuples.length; i++) {
var tuple = tuples[i];
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/facade/collection.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ class StringMapWrapper {
class ListWrapper {
static List clone(Iterable l) => new List.from(l);
static List createFixedSize(int size) => new List(size);
static List createGrowableSize(int size) =>
new List.generate(size, (_) => null, growable: true);
static get(List m, int k) => m[k];
static void set(List m, int k, v) {
m[k] = v;
Expand Down
3 changes: 3 additions & 0 deletions modules/angular2/src/facade/collection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ export class StringMapWrapper {
}

export class ListWrapper {
// JS has no way to express a staticly fixed size list, but dart does so we
// keep both methods.
static createFixedSize(size): List<any> { return new List(size); }
static createGrowableSize(size): List<any> { return new List(size); }
static get(m, k) { return m[k]; }
static set(m, k, v) { m[k] = v; }
static clone(array: List<any>) { return array.slice(0); }
Expand Down
11 changes: 4 additions & 7 deletions modules/angular2/test/core/compiler/query_integration_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,11 @@ export function main() {
});
}));

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

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

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

tb.createView(MyComp, {html: template})
.then((view) => {
Expand Down
54 changes: 39 additions & 15 deletions modules/angular2/test/core/compiler/view_manager_utils_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import {ElementBinder} from 'angular2/src/core/compiler/element_binder';
import {
DirectiveBinding,
ElementInjector,
PreBuiltObjects
PreBuiltObjects,
ProtoElementInjector
} from 'angular2/src/core/compiler/element_injector';
import {DirectiveResolver} from 'angular2/src/core/compiler/directive_resolver';
import {Component} from 'angular2/annotations';
Expand Down Expand Up @@ -66,10 +67,12 @@ export function main() {
return res;
}

function createElementInjector() {
function createElementInjector(parent = null) {
var host = new SpyElementInjector();
var appInjector = new SpyInjector();
return SpyObject.stub(new SpyElementInjector(),
var elementInjector =
isPresent(parent) ? new SpyElementInjectorWithParent(parent) : new SpyElementInjector();
return SpyObject.stub(elementInjector,
{
'isExportingComponent': false,
'isExportingElement': false,
Expand All @@ -82,15 +85,19 @@ export function main() {
{});
}

function createView(pv = null) {
function createView(pv = null, nestedInjectors = false) {
if (isBlank(pv)) {
pv = createProtoView();
}
var view = new AppView(null, pv, new Map());
var elementInjectors = ListWrapper.createFixedSize(pv.elementBinders.length);
var elementInjectors = ListWrapper.createGrowableSize(pv.elementBinders.length);
var preBuiltObjects = ListWrapper.createFixedSize(pv.elementBinders.length);
for (var i = 0; i < pv.elementBinders.length; i++) {
elementInjectors[i] = createElementInjector();
if (nestedInjectors && i > 0) {
elementInjectors[i] = createElementInjector(elementInjectors[i - 1]);
} else {
elementInjectors[i] = createElementInjector();
}
preBuiltObjects[i] = new SpyPreBuiltObjects();
}
view.init(<any>new SpyChangeDetector(), elementInjectors, elementInjectors, preBuiltObjects,
Expand Down Expand Up @@ -118,10 +125,9 @@ export function main() {
var spyCd = <any>componentView.changeDetector;
spyCd.spy('hydrate').andCallFake(log.fn('hydrateCD'));

utils.hydrateComponentView(hostView, 0)
utils.hydrateComponentView(hostView, 0);

expect(log.result())
.toEqual('hydrate; hydrateCD');
expect(log.result()).toEqual('hydrate; hydrateCD');
});

});
Expand Down Expand Up @@ -187,25 +193,32 @@ export function main() {
describe('attachViewInContainer', () => {
var parentView, contextView, childView;

function createViews() {
function createViews(numInj = 1) {
var parentPv = createProtoView([createEmptyElBinder()]);
parentView = createView(parentPv);

var contextPv = createProtoView([createEmptyElBinder()]);
contextView = createView(contextPv);
var binders = [];
for (var i = 0; i < numInj; i++) binders.push(createEmptyElBinder());
var contextPv = createProtoView(binders);
contextView = createView(contextPv, true);

var childPv = createProtoView([createEmptyElBinder()]);
childView = createView(childPv);
}

it('should link the views rootElementInjectors at the given context', () => {
createViews();
utils.attachViewInContainer(parentView, 0, contextView, 0, 0, childView);
expect(contextView.elementInjectors.length).toEqual(2);
});

it('should link the views rootElementInjectors after the elementInjector at the given context',
() => {
createViews();
utils.attachViewInContainer(parentView, 0, contextView, 0, 0, childView);
createViews(2);
utils.attachViewInContainer(parentView, 0, contextView, 1, 0, childView);
expect(childView.rootElementInjectors[0].spy('linkAfter'))
.toHaveBeenCalledWith(contextView.elementInjectors[0], null);
});

});

describe('hydrateViewInContainer', () => {
Expand Down Expand Up @@ -279,6 +292,17 @@ class SpyElementInjector extends SpyObject {
noSuchMethod(m) { return super.noSuchMethod(m) }
}

@proxy
@IMPLEMENTS(ElementInjector)
class SpyElementInjectorWithParent extends SpyObject {
parent: ElementInjector;
constructor(parent) {
super(ElementInjector);
this.parent = parent;
}
noSuchMethod(m) { return super.noSuchMethod(m) }
}

@proxy
@IMPLEMENTS(PreBuiltObjects)
class SpyPreBuiltObjects extends SpyObject {
Expand Down

0 comments on commit d800d2f

Please sign in to comment.