From 4efc4a552044336f44731dccb11db42b00edc6ad Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 23 Sep 2015 08:45:21 -0700 Subject: [PATCH] feat(query): remove the 3-query-per-element limit Closes #4336 --- .../src/core/compiler/element_injector.ts | 400 +++++++++--------- .../core/compiler/query_integration_spec.ts | 32 +- 2 files changed, 239 insertions(+), 193 deletions(-) diff --git a/modules/angular2/src/core/compiler/element_injector.ts b/modules/angular2/src/core/compiler/element_injector.ts index 59fe3496cd..1bb72ba82c 100644 --- a/modules/angular2/src/core/compiler/element_injector.ts +++ b/modules/angular2/src/core/compiler/element_injector.ts @@ -242,10 +242,30 @@ function _createEventEmitterAccessors(bwv: BindingWithVisibility): EventEmitterA }); } +function _createProtoQueryRefs(bindings: BindingWithVisibility[]): ProtoQueryRef[] { + var res = []; + ListWrapper.forEachWithIndex(bindings, (b, i) => { + if (b.binding instanceof DirectiveBinding) { + // field queries + var queries: QueryMetadataWithSetter[] = b.binding.queries; + queries.forEach(q => res.push(new ProtoQueryRef(i, q.setter, q.metadata))); + + // queries passed into the constructor. + // TODO: remove this after constructor queries are no longer supported + var deps: DirectiveDependency[] = b.binding.resolvedFactories[0].dependencies; + deps.forEach(d => { + if (isPresent(d.queryDecorator)) res.push(new ProtoQueryRef(i, null, d.queryDecorator)); + }); + } + }); + return res; +} + export class ProtoElementInjector { view: viewModule.AppView; attributes: Map; eventEmitterAccessors: EventEmitterAccessor[][]; + protoQueryRefs: ProtoQueryRef[]; protoInjector: ProtoInjector; static create(parent: ProtoElementInjector, index: number, bindings: DirectiveBinding[], @@ -312,6 +332,7 @@ export class ProtoElementInjector { for (var i = 0; i < length; ++i) { this.eventEmitterAccessors[i] = _createEventEmitterAccessors(bwv[i]); } + this.protoQueryRefs = _createProtoQueryRefs(bwv); } instantiate(parent: ElementInjector): ElementInjector { @@ -332,11 +353,7 @@ class _Context { export class ElementInjector extends TreeNode implements DependencyProvider { private _host: ElementInjector; private _preBuiltObjects: PreBuiltObjects = null; - - // QueryRefs are added during construction. They are never removed. - private _query0: QueryRef; - private _query1: QueryRef; - private _query2: QueryRef; + private _queryStrategy: _QueryStrategy; hydrated: boolean; @@ -357,7 +374,7 @@ export class ElementInjector extends TreeNode implements Depend this.hydrated = false; - this._buildQueries(); + this._queryStrategy = this._buildQueryStrategy(); } dehydrate(): void { @@ -366,7 +383,7 @@ export class ElementInjector extends TreeNode implements Depend this._preBuiltObjects = null; this._strategy.callOnDestroy(); this._strategy.dehydrate(); - this._clearQueryLists(); + this._queryStrategy.clearQueryLists(); } hydrate(imperativelyCreatedInjector: Injector, host: ElementInjector, @@ -380,36 +397,6 @@ export class ElementInjector extends TreeNode implements Depend this.hydrated = true; } - updateLocalQueries() { - if (isPresent(this._query0) && !this._query0.isViewQuery) { - this._query0.update(); - this._query0.list.fireCallbacks(); - } - if (isPresent(this._query1) && !this._query1.isViewQuery) { - this._query1.update(); - this._query1.list.fireCallbacks(); - } - if (isPresent(this._query2) && !this._query2.isViewQuery) { - this._query2.update(); - this._query2.list.fireCallbacks(); - } - } - - updateLocalViewQueries() { - if (isPresent(this._query0) && this._query0.isViewQuery) { - this._query0.update(); - this._query0.list.fireCallbacks(); - } - if (isPresent(this._query1) && this._query1.isViewQuery) { - this._query1.update(); - this._query1.list.fireCallbacks(); - } - if (isPresent(this._query2) && this._query2.isViewQuery) { - this._query2.update(); - this._query2.list.fireCallbacks(); - } - } - private _debugContext(): any { var p = this._preBuiltObjects; var index = p.elementRef.boundElementIndex - p.view.elementOffset; @@ -504,7 +491,8 @@ export class ElementInjector extends TreeNode implements Depend if (isPresent(dirDep.attributeName)) return this._buildAttribute(dirDep); - if (isPresent(dirDep.queryDecorator)) return this._findQuery(dirDep.queryDecorator).list; + if (isPresent(dirDep.queryDecorator)) + return this._queryStrategy.findQuery(dirDep.queryDecorator).list; if (dirDep.key.id === StaticKeys.instance().changeDetectorRefId) { // We provide the component's view change detector to components and @@ -557,34 +545,6 @@ export class ElementInjector extends TreeNode implements Depend } } - _buildQueriesForDeps(deps: DirectiveDependency[]): void { - for (var i = 0; i < deps.length; i++) { - var dep = deps[i]; - if (isPresent(dep.queryDecorator)) { - this._createQueryRef(null, null, dep.queryDecorator); - } - } - } - _buildQueriesForDirective(dirIndex: number, meta: QueryMetadataWithSetter[]): void { - for (var i = 0; i < meta.length; i++) { - var m = meta[i]; - this._createQueryRef(dirIndex, m.setter, m.metadata); - } - } - - private _createQueryRef(dirIndex: number, setter: SetterFn, query: QueryMetadata): void { - var queryList = new QueryList(); - if (isBlank(this._query0)) { - this._query0 = new QueryRef(dirIndex, setter, query, queryList, this); - } else if (isBlank(this._query1)) { - this._query1 = new QueryRef(dirIndex, setter, query, queryList, this); - } else if (isBlank(this._query2)) { - this._query2 = new QueryRef(dirIndex, setter, query, queryList, this); - } else { - throw new QueryError(); - } - } - addDirectivesMatchingQuery(query: QueryMetadata, list: any[]): void { var templateRef = isBlank(this._preBuiltObjects) ? null : this._preBuiltObjects.templateRef; if (query.selector === TemplateRef && isPresent(templateRef)) { @@ -593,25 +553,17 @@ export class ElementInjector extends TreeNode implements Depend this._strategy.addDirectivesMatchingQuery(query, list); } - private _buildQueries(): void { - if (isPresent(this._proto)) { - this._strategy.buildQueries(); + private _buildQueryStrategy(): _QueryStrategy { + if (this._proto.protoQueryRefs.length === 0) { + return _emptyQueryStrategy; + } else if (this._proto.protoQueryRefs.length <= + InlineQueryStrategy.NUMBER_OF_SUPPORTED_QUERIES) { + return new InlineQueryStrategy(this); + } else { + return new DynamicQueryStrategy(this); } } - private _findQuery(query): QueryRef { - if (isPresent(this._query0) && this._query0.query === query) { - return this._query0; - } - if (isPresent(this._query1) && this._query1.query === query) { - return this._query1; - } - if (isPresent(this._query2) && this._query2.query === query) { - return this._query2; - } - throw new BaseException(`Cannot find query for directive ${query}.`); - } - link(parent: ElementInjector): void { parent.addChild(this); } unlink(): void { this.remove(); } @@ -631,15 +583,9 @@ export class ElementInjector extends TreeNode implements Depend return isPresent(nestedView) ? nestedView.rootElementInjectors : []; } - private _clearQueryLists(): void { - if (isPresent(this._query0)) this._query0.reset(); - if (isPresent(this._query1)) this._query1.reset(); - if (isPresent(this._query2)) this._query2.reset(); - } + afterViewChecked(): void { this._queryStrategy.updateViewQueries(); } - afterViewChecked(): void { this.updateLocalViewQueries(); } - - afterContentChecked(): void { this.updateLocalQueries(); } + afterContentChecked(): void { this._queryStrategy.updateContentQueries(); } traverseAndSetQueriesAsDirty(): void { var inj = this; @@ -650,16 +596,165 @@ export class ElementInjector extends TreeNode implements Depend } private _setQueriesAsDirty(): void { - if (isPresent(this._query0) && !this._query0.isViewQuery) this._query0.dirty = true; - if (isPresent(this._query1) && !this._query1.isViewQuery) this._query1.dirty = true; - if (isPresent(this._query2) && !this._query2.isViewQuery) this._query2.dirty = true; - if (isPresent(this._host)) this._host._setViewQueriesAsDirty(); + this._queryStrategy.setContentQueriesAsDirty(); + if (isPresent(this._host)) this._host._queryStrategy.setViewQueriesAsDirty(); + } +} + +interface _QueryStrategy { + setContentQueriesAsDirty(): void; + setViewQueriesAsDirty(): void; + clearQueryLists(): void; + updateContentQueries(): void; + updateViewQueries(): void; + findQuery(query: QueryMetadata): QueryRef; +} + +class _EmptyQueryStrategy implements _QueryStrategy { + setContentQueriesAsDirty(): void {} + setViewQueriesAsDirty(): void {} + clearQueryLists(): void {} + updateContentQueries(): void {} + updateViewQueries(): void {} + findQuery(query: QueryMetadata): QueryRef { + throw new BaseException(`Cannot find query for directive ${query}.`); + } +} + +var _emptyQueryStrategy = new _EmptyQueryStrategy(); + +class InlineQueryStrategy implements _QueryStrategy { + static NUMBER_OF_SUPPORTED_QUERIES = 3; + + query0: QueryRef; + query1: QueryRef; + query2: QueryRef; + + constructor(ei: ElementInjector) { + var protoRefs = ei._proto.protoQueryRefs; + if (protoRefs.length > 0) this.query0 = new QueryRef(protoRefs[0], new QueryList(), ei); + if (protoRefs.length > 1) this.query1 = new QueryRef(protoRefs[1], new QueryList(), ei); + if (protoRefs.length > 2) this.query2 = new QueryRef(protoRefs[2], new QueryList(), ei); } - private _setViewQueriesAsDirty(): void { - if (isPresent(this._query0) && this._query0.isViewQuery) this._query0.dirty = true; - if (isPresent(this._query1) && this._query1.isViewQuery) this._query1.dirty = true; - if (isPresent(this._query2) && this._query2.isViewQuery) this._query2.dirty = true; + setContentQueriesAsDirty(): void { + if (isPresent(this.query0) && !this.query0.isViewQuery) this.query0.dirty = true; + if (isPresent(this.query1) && !this.query1.isViewQuery) this.query1.dirty = true; + if (isPresent(this.query2) && !this.query2.isViewQuery) this.query2.dirty = true; + } + + setViewQueriesAsDirty(): void { + if (isPresent(this.query0) && this.query0.isViewQuery) this.query0.dirty = true; + if (isPresent(this.query1) && this.query1.isViewQuery) this.query1.dirty = true; + if (isPresent(this.query2) && this.query2.isViewQuery) this.query2.dirty = true; + } + + clearQueryLists(): void { + if (isPresent(this.query0)) this.query0.reset(); + if (isPresent(this.query1)) this.query1.reset(); + if (isPresent(this.query2)) this.query2.reset(); + } + + updateContentQueries() { + if (isPresent(this.query0) && !this.query0.isViewQuery) { + this.query0.update(); + this.query0.list.fireCallbacks(); + } + if (isPresent(this.query1) && !this.query1.isViewQuery) { + this.query1.update(); + this.query1.list.fireCallbacks(); + } + if (isPresent(this.query2) && !this.query2.isViewQuery) { + this.query2.update(); + this.query2.list.fireCallbacks(); + } + } + + updateViewQueries() { + if (isPresent(this.query0) && this.query0.isViewQuery) { + this.query0.update(); + this.query0.list.fireCallbacks(); + } + if (isPresent(this.query1) && this.query1.isViewQuery) { + this.query1.update(); + this.query1.list.fireCallbacks(); + } + if (isPresent(this.query2) && this.query2.isViewQuery) { + this.query2.update(); + this.query2.list.fireCallbacks(); + } + } + + findQuery(query: QueryMetadata): QueryRef { + if (isPresent(this.query0) && this.query0.protoQueryRef.query === query) { + return this.query0; + } + if (isPresent(this.query1) && this.query1.protoQueryRef.query === query) { + return this.query1; + } + if (isPresent(this.query2) && this.query2.protoQueryRef.query === query) { + return this.query2; + } + throw new BaseException(`Cannot find query for directive ${query}.`); + } +} + +class DynamicQueryStrategy implements _QueryStrategy { + queries: QueryRef[]; + + constructor(ei: ElementInjector) { + this.queries = ei._proto.protoQueryRefs.map(p => new QueryRef(p, new QueryList(), ei)); + } + + setContentQueriesAsDirty(): void { + for (var i = 0; i < this.queries.length; ++i) { + var q = this.queries[i]; + if (!q.isViewQuery) q.dirty = true; + } + } + + setViewQueriesAsDirty(): void { + for (var i = 0; i < this.queries.length; ++i) { + var q = this.queries[i]; + if (q.isViewQuery) q.dirty = true; + } + } + + clearQueryLists(): void { + for (var i = 0; i < this.queries.length; ++i) { + var q = this.queries[i]; + q.reset(); + } + } + + updateContentQueries() { + for (var i = 0; i < this.queries.length; ++i) { + var q = this.queries[i]; + if (!q.isViewQuery) { + q.update(); + q.list.fireCallbacks(); + } + } + } + + updateViewQueries() { + for (var i = 0; i < this.queries.length; ++i) { + var q = this.queries[i]; + if (q.isViewQuery) { + q.update(); + q.list.fireCallbacks(); + } + } + } + + findQuery(query: QueryMetadata): QueryRef { + for (var i = 0; i < this.queries.length; ++i) { + var q = this.queries[i]; + if (q.protoQueryRef.query === query) { + return q; + } + } + throw new BaseException(`Cannot find query for directive ${query}.`); } } @@ -667,7 +762,6 @@ interface _ElementInjectorStrategy { callOnDestroy(): void; getComponent(): any; isComponentKey(key: Key): boolean; - buildQueries(): void; addDirectivesMatchingQuery(q: QueryMetadata, res: any[]): void; hydrate(): void; dehydrate(): void; @@ -765,63 +859,6 @@ class ElementInjectorInlineStrategy implements _ElementInjectorStrategy { key.id === this.injectorStrategy.protoStrategy.keyId0; } - buildQueries(): void { - var p = this.injectorStrategy.protoStrategy; - - if (p.binding0 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding0.resolvedFactories[0].dependencies); - - this._ei._buildQueriesForDirective(0, (p.binding0).queries); - } - if (p.binding1 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding1.resolvedFactories[0].dependencies); - - this._ei._buildQueriesForDirective(1, (p.binding1).queries); - } - if (p.binding2 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding2.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(2, (p.binding2).queries); - } - if (p.binding3 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding3.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(3, (p.binding3).queries); - } - if (p.binding4 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding4.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(4, (p.binding4).queries); - } - if (p.binding5 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding5.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(5, (p.binding5).queries); - } - if (p.binding6 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding6.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(6, (p.binding6).queries); - } - if (p.binding7 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding7.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(7, (p.binding7).queries); - } - if (p.binding8 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding8.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(8, (p.binding8).queries); - } - if (p.binding9 instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.binding9.resolvedFactories[0].dependencies); - this._ei._buildQueriesForDirective(9, (p.binding9).queries); - } - } - addDirectivesMatchingQuery(query: QueryMetadata, list: any[]): void { var i = this.injectorStrategy; var p = i.protoStrategy; @@ -913,19 +950,6 @@ class ElementInjectorDynamicStrategy implements _ElementInjectorStrategy { return this._ei._proto._firstBindingIsComponent && isPresent(key) && key.id === p.keyIds[0]; } - buildQueries(): void { - var inj = this.injectorStrategy; - var p = inj.protoStrategy; - - for (var i = 0; i < p.bindings.length; i++) { - if (p.bindings[i] instanceof DirectiveBinding) { - this._ei._buildQueriesForDeps( - p.bindings[i].resolvedFactory.dependencies); - this._ei._buildQueriesForDirective(i, (p.bindings[i]).queries); - } - } - } - addDirectivesMatchingQuery(query: QueryMetadata, list: any[]): void { var ist = this.injectorStrategy; var p = ist.protoStrategy; @@ -941,23 +965,17 @@ class ElementInjectorDynamicStrategy implements _ElementInjectorStrategy { } } -export class QueryError extends BaseException { - message: string; - // TODO(rado): pass the names of the active directives. - constructor() { - super(); - this.message = 'Only 3 queries can be concurrently active on an element.'; - } +export class ProtoQueryRef { + constructor(public dirIndex: number, public setter: SetterFn, public query: QueryMetadata) {} - toString(): string { return this.message; } + get usesPropertySyntax(): boolean { return isPresent(this.setter); } } export class QueryRef { - constructor(public dirIndex: number, public setter: SetterFn, public query: QueryMetadata, - public list: QueryList, public originator: ElementInjector, - public dirty: boolean = true) {} + constructor(public protoQueryRef: ProtoQueryRef, public list: QueryList, + private originator: ElementInjector, public dirty: boolean = true) {} - get isViewQuery(): boolean { return this.query.isViewQuery; } + get isViewQuery(): boolean { return this.protoQueryRef.query.isViewQuery; } update(): void { if (!this.dirty) return; @@ -965,19 +983,19 @@ export class QueryRef { this.dirty = false; // TODO delete the check once only field queries are supported - if (isPresent(this.dirIndex)) { - var dir = this.originator.getDirectiveAtIndex(this.dirIndex); - if (this.query.first) { - this.setter(dir, this.list.length > 0 ? this.list.first : null); + if (this.protoQueryRef.usesPropertySyntax) { + var dir = this.originator.getDirectiveAtIndex(this.protoQueryRef.dirIndex); + if (this.protoQueryRef.query.first) { + this.protoQueryRef.setter(dir, this.list.length > 0 ? this.list.first : null); } else { - this.setter(dir, this.list); + this.protoQueryRef.setter(dir, this.list); } } } private _update(): void { var aggregator = []; - if (this.query.isViewQuery) { + if (this.protoQueryRef.query.isViewQuery) { var view = this.originator.getView(); // intentionally skipping originator for view queries. var nestedView = @@ -1002,7 +1020,7 @@ export class QueryRef { break; } - if (!this.query.descendants && + if (!this.protoQueryRef.query.descendants && !(curInj.parent == this.originator || curInj == this.originator)) continue; @@ -1017,7 +1035,7 @@ export class QueryRef { } private _visitInjector(inj: ElementInjector, aggregator: any[]) { - if (this.query.isVarBindingQuery) { + if (this.protoQueryRef.query.isVarBindingQuery) { this._aggregateVariableBindings(inj, aggregator); } else { this._aggregateDirective(inj, aggregator); @@ -1043,7 +1061,7 @@ export class QueryRef { } private _aggregateVariableBindings(inj: ElementInjector, aggregator: any[]): void { - var vb = this.query.varBindings; + var vb = this.protoQueryRef.query.varBindings; for (var i = 0; i < vb.length; ++i) { if (inj.hasVariableBinding(vb[i])) { aggregator.push(inj.getVariableBinding(vb[i])); @@ -1052,7 +1070,7 @@ export class QueryRef { } private _aggregateDirective(inj: ElementInjector, aggregator: any[]): void { - inj.addDirectivesMatchingQuery(this.query, aggregator); + inj.addDirectivesMatchingQuery(this.protoQueryRef.query, aggregator); } reset(): void { diff --git a/modules/angular2/test/core/compiler/query_integration_spec.ts b/modules/angular2/test/core/compiler/query_integration_spec.ts index f2a45e11ed..00c9c1ca5d 100644 --- a/modules/angular2/test/core/compiler/query_integration_spec.ts +++ b/modules/angular2/test/core/compiler/query_integration_spec.ts @@ -43,7 +43,6 @@ import {BrowserDomAdapter} from 'angular2/src/core/dom/browser_adapter'; export function main() { BrowserDomAdapter.makeCurrent(); describe('Query API', () => { - describe("querying by directive type", () => { it('should contain all direct child directives in the light dom (constructor)', inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { @@ -646,6 +645,25 @@ export function main() { expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', newString, '4']); } + async.done(); + }); + })); + + it('should support more than three queries', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = '
'; + + tcb.overrideTemplate(MyComp, template) + .createAsync(MyComp) + .then((view) => { + view.detectChanges(); + + var q = view.debugElement.componentViewChildren[0].getLocal('q'); + expect(q.query1).toBeDefined(); + expect(q.query2).toBeDefined(); + expect(q.query3).toBeDefined(); + expect(q.query4).toBeDefined(); + async.done(); }); })); @@ -743,6 +761,15 @@ class NeedsQuery { constructor(@Query(TextDirective) query: QueryList) { this.query = query; } } +@Component({selector: 'needs-four-queries'}) +@View({template: ''}) +class NeedsFourQueries { + @ContentChild(TextDirective) query1: TextDirective; + @ContentChild(TextDirective) query2: TextDirective; + @ContentChild(TextDirective) query3: TextDirective; + @ContentChild(TextDirective) query4: TextDirective; +} + @Component({selector: 'needs-query-desc'}) @View({directives: [NgFor], template: '
{{dir.text}}|
'}) @Injectable() @@ -900,7 +927,8 @@ class NeedsTpl { TextDirective, InertDirective, NgIf, - NgFor + NgFor, + NeedsFourQueries ] }) @Injectable()