From 4943c0f8872a691a78d2e8fda3c7ea19619420fd Mon Sep 17 00:00:00 2001 From: vsavkin Date: Fri, 17 Apr 2015 16:08:59 -0700 Subject: [PATCH] fix(view): fixed hydrator to pass the right element index when attaching an event listener --- .../src/core/compiler/element_injector.js | 7 ++ .../src/core/compiler/view_hydrator.js | 13 +--- modules/angular2/src/facade/collection.dart | 4 +- modules/angular2/src/test_lib/test_lib.dart | 38 ++++++++-- modules/angular2/src/test_lib/test_lib.es6 | 16 ++++ .../test/core/compiler/view_hydrator_spec.js | 73 +++++++++++++------ .../angular2/test/test_lib/test_lib_spec.js | 7 ++ 7 files changed, 121 insertions(+), 37 deletions(-) diff --git a/modules/angular2/src/core/compiler/element_injector.js b/modules/angular2/src/core/compiler/element_injector.js index 8674e03636..060398bd47 100644 --- a/modules/angular2/src/core/compiler/element_injector.js +++ b/modules/angular2/src/core/compiler/element_injector.js @@ -1,4 +1,5 @@ import {isPresent, isBlank, Type, int, BaseException} from 'angular2/src/facade/lang'; +import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; import {Math} from 'angular2/src/facade/math'; import {List, ListWrapper, MapWrapper} from 'angular2/src/facade/collection'; import {Injector, Key, Dependency, bind, Binding, ResolvedBinding, NoBindingError, @@ -318,6 +319,12 @@ class EventEmitterAccessor { this.eventName = eventName; this.getter = getter; } + + subscribe(view:viewModule.AppView, boundElementIndex:number, directive:Object) { + var eventEmitter = this.getter(directive); + return ObservableWrapper.subscribe(eventEmitter, + eventObj => view.triggerEventHandlers(this.eventName, eventObj, boundElementIndex)); + } } /** diff --git a/modules/angular2/src/core/compiler/view_hydrator.js b/modules/angular2/src/core/compiler/view_hydrator.js index c0072aeee5..59055d0faa 100644 --- a/modules/angular2/src/core/compiler/view_hydrator.js +++ b/modules/angular2/src/core/compiler/view_hydrator.js @@ -2,7 +2,6 @@ import {Injectable, Inject, OpaqueToken, Injector} from 'angular2/di'; import {ListWrapper, MapWrapper, Map, StringMapWrapper, List} from 'angular2/src/facade/collection'; import * as eli from './element_injector'; import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang'; -import {ObservableWrapper} from 'angular2/src/facade/async'; import * as vcModule from './view_container'; import * as viewModule from './view'; import {BindingPropagationConfig, Locals} from 'angular2/change_detection'; @@ -160,7 +159,7 @@ export class AppViewHydrator { var elementInjector = view.elementInjectors[i]; if (isPresent(elementInjector)) { elementInjector.instantiateDirectives(appInjector, hostElementInjector, shadowDomAppInjector, view.preBuiltObjects[i]); - this._setUpEventEmitters(view, elementInjector); + this._setUpEventEmitters(view, elementInjector, i); // The exporting of $implicit is a special case. Since multiple elements will all export // the different values as $implicit, directly assign $implicit bindings to the variable @@ -190,7 +189,7 @@ export class AppViewHydrator { return renderComponentIndex; } - _setUpEventEmitters(view:viewModule.AppView, elementInjector:eli.ElementInjector) { + _setUpEventEmitters(view:viewModule.AppView, elementInjector:eli.ElementInjector, boundElementIndex:number) { var emitters = elementInjector.getEventEmitterAccessors(); for(var directiveIndex = 0; directiveIndex < emitters.length; ++directiveIndex) { var directiveEmitters = emitters[directiveIndex]; @@ -198,17 +197,11 @@ export class AppViewHydrator { for (var eventIndex = 0; eventIndex < directiveEmitters.length; ++eventIndex) { var eventEmitterAccessor = directiveEmitters[eventIndex]; - this._setUpSubscription(view, directive, directiveIndex, eventEmitterAccessor); + eventEmitterAccessor.subscribe(view, boundElementIndex, directive); } } } - _setUpSubscription(view:viewModule.AppView, directive:Object, directiveIndex:number, eventEmitterAccessor) { - var eventEmitter = eventEmitterAccessor.getter(directive); - ObservableWrapper.subscribe(eventEmitter, - eventObj => view.triggerEventHandlers(eventEmitterAccessor.eventName, eventObj, directiveIndex)); - } - /** * This should only be called by View or ViewContainer. */ diff --git a/modules/angular2/src/facade/collection.dart b/modules/angular2/src/facade/collection.dart index 98a8fcc846..3655a17243 100644 --- a/modules/angular2/src/facade/collection.dart +++ b/modules/angular2/src/facade/collection.dart @@ -77,7 +77,9 @@ class StringMapWrapper { } static Map merge(Map a, Map b) { var m = new Map.from(a); - b.forEach((k, v) => m[k] = v); + if (b != null) { + b.forEach((k, v) => m[k] = v); + } return m; } static bool isEmpty(Map m) => m.isEmpty; diff --git a/modules/angular2/src/test_lib/test_lib.dart b/modules/angular2/src/test_lib/test_lib.dart index c8eeba7c83..9b3417ad07 100644 --- a/modules/angular2/src/test_lib/test_lib.dart +++ b/modules/angular2/src/test_lib/test_lib.dart @@ -1,7 +1,7 @@ library test_lib.test_lib; import 'package:guinness/guinness.dart' as gns; -export 'package:guinness/guinness.dart' hide Expect, expect, NotExpect, beforeEach, it, iit, xit, SpyObject; +export 'package:guinness/guinness.dart' hide Expect, expect, NotExpect, beforeEach, it, iit, xit; import 'package:unittest/unittest.dart' hide expect; import 'dart:async'; @@ -13,6 +13,7 @@ import 'package:angular2/src/reflection/reflection_capabilities.dart'; import 'package:angular2/src/di/binding.dart' show bind; import 'package:angular2/src/di/injector.dart' show Injector; +import 'package:angular2/src/facade/collection.dart' show StringMapWrapper; import './test_injector.dart'; export './test_injector.dart' show inject; @@ -149,13 +150,40 @@ xit(name, fn) { _it(gns.xit, name, fn); } -class SpyObject extends gns.SpyObject { - // Need to take an optional type as this is required by - // the JS SpyObject. - SpyObject([type = null]) { +class SpyFunction extends gns.SpyFunction { + SpyFunction(name): super(name); + + // TODO: vsavkin move to guinness + andReturn(value) { + return andCallFake(([a0, a1, a2, a3, a4, a5]) => value); } } +class SpyObject extends gns.SpyObject { + final Map _spyFuncs = {}; + + SpyObject([arg]){} + + SpyFunction spy(String funcName) => + _spyFuncs.putIfAbsent(funcName, () => new SpyFunction(funcName)); + + static stub([object = null, config = null, overrides = null]) { + if (object is! SpyObject) { + overrides = config; + config = object; + object = new SpyObject(); + } + + var m = StringMapWrapper.merge(config, overrides); + StringMapWrapper.forEach(m, (value, key){ + object.spy(key).andReturn(value); + }); + return object; + } +} + + + String elementText(n) { hasNodes(n) { var children = DOM.childNodes(n); diff --git a/modules/angular2/src/test_lib/test_lib.es6 b/modules/angular2/src/test_lib/test_lib.es6 index b3227fc2c4..1d3f2f5845 100644 --- a/modules/angular2/src/test_lib/test_lib.es6 +++ b/modules/angular2/src/test_lib/test_lib.es6 @@ -1,4 +1,5 @@ import {DOM} from 'angular2/src/dom/dom_adapter'; +import {StringMapWrapper} from 'angular2/src/facade/collection'; import {bind} from 'angular2/di'; @@ -289,6 +290,20 @@ export class SpyObject { return this[name]; } + static stub(object = null, config = null, overrides = null) { + if (!(object instanceof SpyObject)) { + overrides = config; + config = object; + object = new SpyObject(); + } + + var m = StringMapWrapper.merge(config, overrides); + StringMapWrapper.forEach(m, (value, key) => { + object.spy(key).andReturn(value); + }); + return object; + } + rttsAssert(value) { return true; } @@ -296,6 +311,7 @@ export class SpyObject { _createGuinnessCompatibleSpy(){ var newSpy = jasmine.createSpy(); newSpy.andCallFake = newSpy.and.callFake; + newSpy.andReturn = newSpy.and.returnValue; // return null by default to satisfy our rtts asserts newSpy.and.returnValue(null); return newSpy; diff --git a/modules/angular2/test/core/compiler/view_hydrator_spec.js b/modules/angular2/test/core/compiler/view_hydrator_spec.js index 9817999825..20e4dbe72c 100644 --- a/modules/angular2/test/core/compiler/view_hydrator_spec.js +++ b/modules/angular2/test/core/compiler/view_hydrator_spec.js @@ -14,8 +14,8 @@ import { xit, SpyObject, proxy } from 'angular2/test_lib'; -import {IMPLEMENTS, isBlank} from 'angular2/src/facade/lang'; -import {MapWrapper, ListWrapper} from 'angular2/src/facade/collection'; +import {IMPLEMENTS, isBlank, isPresent} from 'angular2/src/facade/lang'; +import {MapWrapper, ListWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import {AppProtoView, AppView} from 'angular2/src/core/compiler/view'; import {Renderer, ViewRef} from 'angular2/src/render/api'; @@ -43,12 +43,13 @@ export function main() { return DirectiveBinding.createFromType(meta.type, meta.annotation); } - function createElementInjector() { - var res = new SpyElementInjector(); - res.spy('isExportingComponent').andCallFake( () => false ); - res.spy('isExportingElement').andCallFake( () => false ); - res.spy('getEventEmitterAccessors').andCallFake( () => [] ); - return res; + function createElementInjector(overrides) { + return SpyObject.stub(new SpyElementInjector(), { + 'isExportingComponent' : false, + 'isExportingElement' : false, + 'getEventEmitterAccessors' : [], + 'getComponent' : null + }, overrides); } function createEmptyElBinder() { @@ -86,13 +87,18 @@ export function main() { return view; } - function createHostView(pv, shadowView, componentInstance) { + function createHostView(pv, shadowView, componentInstance, elementInjectors = null) { var view = new AppView(renderer, null, null, pv, MapWrapper.create()); var changeDetector = new SpyChangeDetector(); - var eij = createElementInjector(); - eij.spy('getComponent').andCallFake( () => componentInstance ); - view.init(changeDetector, [eij], [eij], - [null], [shadowView]); + + var eis; + if (isPresent(elementInjectors)) { + eis = elementInjectors; + } else { + eis = [createElementInjector({'getComponent': componentInstance})]; + } + + view.init(changeDetector, eis, eis, ListWrapper.createFixedSize(eis.length), [shadowView]); return view; } @@ -128,9 +134,7 @@ export function main() { var pv = createHostProtoView(null); var shadowView = createEmptyView(); var view = createHostView(pv, null, null); - renderer.spy('createDynamicComponentView').andCallFake( (a,b,c) => { - return [new ViewRef(), new ViewRef()]; - }); + renderer.spy('createDynamicComponentView').andReturn([new ViewRef(), new ViewRef()]); hydrator.hydrateDynamicComponentView(view, 0, shadowView, createDirectiveBinding(SomeComponent), null); expect( () => hydrator.hydrateDynamicComponentView(view, 0, shadowView, null, null) @@ -143,7 +147,7 @@ export function main() { it('should hydrate existing child components', () => { var hostPv = createHostProtoView(createProtoView()); - var componentInstance = {}; + var componentInstance = new Object(); var shadowView = createEmptyView(); var hostView = createHostView(hostPv, shadowView, componentInstance); renderer.spy('createInPlaceHostView').andCallFake( (a,b,c) => { @@ -155,6 +159,35 @@ export function main() { expect(shadowView.hydrated()).toBe(true); }); + it("should set up event listeners", () => { + var dir = new Object(); + + var hostPv = createProtoView([ + createComponentElBinder(createDirectiveBinding(SomeComponent)), + createEmptyElBinder() + ]); + + var spyEventAccessor1 = SpyObject.stub({"subscribe" : null}); + var ei1 = createElementInjector({ + 'getEventEmitterAccessors': [[spyEventAccessor1]], + 'getDirectiveAtIndex': dir + }); + + var spyEventAccessor2 = SpyObject.stub({"subscribe" : null}); + var ei2 = createElementInjector({ + 'getEventEmitterAccessors': [[spyEventAccessor2]], + 'getDirectiveAtIndex': dir + }); + + var shadowView = createEmptyView(); + var hostView = createHostView(hostPv, shadowView, null, [ei1, ei2]); + renderer.spy('createInPlaceHostView').andReturn([new ViewRef(), new ViewRef()]); + + hydrate(hostView); + + expect(spyEventAccessor1.spy('subscribe')).toHaveBeenCalledWith(hostView, 0, dir); + expect(spyEventAccessor2.spy('subscribe')).toHaveBeenCalledWith(hostView, 1, dir); + }); }); describe('dehydrate... shared functionality', () => { @@ -162,13 +195,11 @@ export function main() { var shadowView; function createAndHydrate(nestedProtoView) { - var componentInstance = {}; + var componentInstance = new Object(); shadowView = createEmptyView(); var hostPv = createHostProtoView(nestedProtoView); hostView = createHostView(hostPv, shadowView, componentInstance); - renderer.spy('createInPlaceHostView').andCallFake( (a,b,c) => { - return [new ViewRef(), new ViewRef()]; - }); + renderer.spy('createInPlaceHostView').andReturn([new ViewRef(), new ViewRef()]); hydrate(hostView); } diff --git a/modules/angular2/test/test_lib/test_lib_spec.js b/modules/angular2/test/test_lib/test_lib_spec.js index 904a7c5d12..3d39112d2d 100644 --- a/modules/angular2/test/test_lib/test_lib_spec.js +++ b/modules/angular2/test/test_lib/test_lib_spec.js @@ -80,6 +80,13 @@ export function main() { expect(spyObj.spy("someFunc")).toHaveBeenCalledWith(1,2); }); + it("should support stubs", () => { + var s = SpyObject.stub({"a":1}, {"b":2}); + + expect(s.a()).toEqual(1); + expect(s.b()).toEqual(2); + }); + it('should create spys for all methods', () => { expect(() => spyObj.someFunc()).not.toThrow(); });