From 60456c8b891ad5d8dd9665a1ce64f4f878a95b40 Mon Sep 17 00:00:00 2001 From: Rado Kirov Date: Fri, 5 Dec 2014 17:44:00 -0800 Subject: [PATCH] feat(ng-repeat): initial implementaion of ng-repeat. - adds support for content bindings via '[]'. - directives module --- .gitignore | 3 +- karma-dart.conf.js | 1 + .../pipeline/element_binder_builder.js | 9 +- modules/core/src/compiler/view.js | 7 +- modules/core/src/compiler/viewport.js | 6 +- modules/core/test/compiler/view_spec.js | 13 +- modules/directives/pubspec.yaml | 18 ++ modules/directives/src/ng_repeat.js | 94 +++++++ modules/directives/test/ng_repeat_spec.js | 239 ++++++++++++++++++ modules/facade/src/dom.dart | 3 + modules/facade/src/dom.es6 | 3 + test-main.js | 3 + tools/build/runtime_paths.js | 3 +- 13 files changed, 388 insertions(+), 14 deletions(-) create mode 100644 modules/directives/pubspec.yaml create mode 100644 modules/directives/src/ng_repeat.js create mode 100644 modules/directives/test/ng_repeat_spec.js diff --git a/.gitignore b/.gitignore index 4362878661..671e6e8b9b 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ node_modules pubspec.lock .c9 .idea/ +*.swo -/docs/bower_components/ \ No newline at end of file +/docs/bower_components/ diff --git a/karma-dart.conf.js b/karma-dart.conf.js index faa3bd13d5..663ff83b99 100644 --- a/karma-dart.conf.js +++ b/karma-dart.conf.js @@ -46,6 +46,7 @@ module.exports = function(config) { '/packages/change_detection': 'http://localhost:9877/base/modules/change_detection/src', '/packages/reflection': 'http://localhost:9877/base/modules/reflection/src', '/packages/di': 'http://localhost:9877/base/modules/di/src', + '/packages/directives': 'http://localhost:9877/base/modules/directives/src', '/packages/facade': 'http://localhost:9877/base/modules/facade/src', '/packages/test_lib': 'http://localhost:9877/base/modules/test_lib/src', }, diff --git a/modules/core/src/compiler/pipeline/element_binder_builder.js b/modules/core/src/compiler/pipeline/element_binder_builder.js index 6b657cd7bd..1716f68f42 100644 --- a/modules/core/src/compiler/pipeline/element_binder_builder.js +++ b/modules/core/src/compiler/pipeline/element_binder_builder.js @@ -118,11 +118,16 @@ export class ElementBinderBuilder extends CompileStep { throw new BaseException('No element binding found for property '+elProp +' which is required by directive '+stringify(typeWithAnnotation.type)); } + var len = dirProp.length; + var dirBindingName = dirProp; + var isContentWatch = dirProp[len - 2] === '[' && dirProp[len - 1] === ']'; + if (isContentWatch) dirBindingName = dirProp.substring(0, len - 2); protoView.bindDirectiveProperty( directiveIndex++, expression, - dirProp, - reflector.setter(dirProp) + dirBindingName, + reflector.setter(dirBindingName), + isContentWatch ); }); }); diff --git a/modules/core/src/compiler/view.js b/modules/core/src/compiler/view.js index 40bbb56dae..36d6aacf94 100644 --- a/modules/core/src/compiler/view.js +++ b/modules/core/src/compiler/view.js @@ -417,7 +417,8 @@ export class ProtoView { directiveIndex:number, expression:AST, setterName:string, - setter:SetterFn) { + setter:SetterFn, + isContentWatch: boolean) { var expMemento = new DirectivePropertyMemento( this.elementBinders.length-1, @@ -426,7 +427,7 @@ export class ProtoView { setter ); var groupMemento = DirectivePropertyGroupMemento.get(expMemento); - this.protoRecordRange.addRecordsFromAST(expression, expMemento, groupMemento, false); + this.protoRecordRange.addRecordsFromAST(expression, expMemento, groupMemento, isContentWatch); } // Create a rootView as if the compiler encountered , @@ -500,7 +501,7 @@ class DirectivePropertyGroupMemento { var directiveIndex = memento._directiveIndex; var id = elementInjectorIndex * 100 + directiveIndex; - if (! MapWrapper.contains(_groups, id)) { + if (!MapWrapper.contains(_groups, id)) { MapWrapper.set(_groups, id, new DirectivePropertyGroupMemento(elementInjectorIndex, directiveIndex)); } return MapWrapper.get(_groups, id); diff --git a/modules/core/src/compiler/viewport.js b/modules/core/src/compiler/viewport.js index fa6ac0a529..e49a8931bd 100644 --- a/modules/core/src/compiler/viewport.js +++ b/modules/core/src/compiler/viewport.js @@ -37,7 +37,11 @@ export class ViewPort { dehydrate() { this.appInjector = null; this.hostElementInjector = null; - for (var i = 0; i < this._views.length; i++) { + this.clear(); + } + + clear() { + for (var i = this._views.length - 1; i >= 0; i--) { this.remove(i); } } diff --git a/modules/core/test/compiler/view_spec.js b/modules/core/test/compiler/view_spec.js index 50e5b1c26b..16431df483 100644 --- a/modules/core/test/compiler/view_spec.js +++ b/modules/core/test/compiler/view_spec.js @@ -382,7 +382,7 @@ export function main() { var pv = new ProtoView(createElement('
'), new ProtoRecordRange()); pv.bindElement(new ProtoElementInjector(null, 0, [SomeDirective])); - pv.bindDirectiveProperty(0, parser.parseBinding('foo'), 'prop', reflector.setter('prop')); + pv.bindDirectiveProperty(0, parser.parseBinding('foo'), 'prop', reflector.setter('prop'), false); createViewAndChangeDetector(pv); ctx.foo = 'buz'; @@ -395,8 +395,8 @@ export function main() { new ProtoRecordRange()); pv.bindElement(new ProtoElementInjector(null, 0, [DirectiveImplementingOnChange])); - pv.bindDirectiveProperty( 0, parser.parseBinding('a'), 'a', reflector.setter('a')); - pv.bindDirectiveProperty( 0, parser.parseBinding('b'), 'b', reflector.setter('b')); + pv.bindDirectiveProperty( 0, parser.parseBinding('a'), 'a', reflector.setter('a'), false); + pv.bindDirectiveProperty( 0, parser.parseBinding('b'), 'b', reflector.setter('b'), false); createViewAndChangeDetector(pv); ctx.a = 100; @@ -412,9 +412,10 @@ export function main() { new ProtoRecordRange()); pv.bindElement(new ProtoElementInjector(null, 0, [DirectiveImplementingOnChange])); - pv.bindDirectiveProperty( 0, parser.parseBinding('a').ast, 'a', reflector.setter('a')); - pv.bindDirectiveProperty( 0, parser.parseBinding('b').ast, 'b', reflector.setter('b')); - createView(pv); + pv.bindDirectiveProperty( 0, parser.parseBinding('a').ast, 'a', reflector.setter('a'), false); + pv.bindDirectiveProperty( 0, parser.parseBinding('b').ast, 'b', reflector.setter('b'), false); + createViewAndChangeDetector(pv); + ctx.a = 0; ctx.b = 0; cd.detectChanges(); diff --git a/modules/directives/pubspec.yaml b/modules/directives/pubspec.yaml new file mode 100644 index 0000000000..58d7b59fda --- /dev/null +++ b/modules/directives/pubspec.yaml @@ -0,0 +1,18 @@ +name: directives +environment: + sdk: '>=1.4.0' +dependencies: + core: + path: ../core + change_detection: + path: ../change_detection + di: + path: ../di + facade: + path: ../facade + reflection: + path: ../reflection +dev_dependencies: + test_lib: + path: ../test_lib + guinness: ">=0.1.16 <0.2.0" diff --git a/modules/directives/src/ng_repeat.js b/modules/directives/src/ng_repeat.js new file mode 100644 index 0000000000..6c51729ae8 --- /dev/null +++ b/modules/directives/src/ng_repeat.js @@ -0,0 +1,94 @@ +import {describe, xit, it, expect, beforeEach, ddescribe, iit} from 'test_lib/test_lib'; + +import {Decorator, Component, Template} from 'core/annotations/annotations'; +import {OnChange} from 'core/compiler/interfaces'; +import {ViewPort} from 'core/compiler/viewport'; +import {View} from 'core/compiler/view'; +import {isPresent, isBlank} from 'facade/lang'; +import {ListWrapper, List} from 'facade/collection'; + +@Template({ + selector: '[ng-repeat]', + bind: { + 'in': 'iterable[]' + } +}) +export class NgRepeat extends OnChange { + viewPort: ViewPort; + iterable; + constructor(viewPort: ViewPort) { + this.viewPort = viewPort; + } + onChange(changes) { + var iteratorChanges = changes['iterable']; + if (isBlank(iteratorChanges) || isBlank(iteratorChanges.currentValue)) { + this.viewPort.clear(); + return; + } + + // TODO(rado): check if change detection can produce a change record that is + // easier to consume than current. + var recordViewTuples = []; + iteratorChanges.currentValue.forEachRemovedItem( + (removedRecord) => ListWrapper.push(recordViewTuples, new RecordViewTuple(removedRecord, null)) + ); + + iteratorChanges.currentValue.forEachMovedItem( + (movedRecord) => ListWrapper.push(recordViewTuples, new RecordViewTuple(movedRecord, null)) + ); + + var insertTuples = NgRepeat.bulkRemove(recordViewTuples, this.viewPort); + + iteratorChanges.currentValue.forEachAddedItem( + (addedRecord) => ListWrapper.push(insertTuples, new RecordViewTuple(addedRecord, null)) + ); + + NgRepeat.bulkInsert(insertTuples, this.viewPort); + + for (var i = 0; i < insertTuples.length; i++) { + this.perViewChange(insertTuples[i].view, insertTuples[i].record); + } + } + + perViewChange(view, record) { + view.setLocal('ng-repeat', record.item); + // Uncomment when binding is ready. + // view.setLocal('index', record.item); + } + + static bulkRemove(tuples, viewPort) { + tuples.sort((a, b) => a.record.previousIndex - b.record.previousIndex); + var movedTuples = []; + for (var i = tuples.length - 1; i >= 0; i--) { + var tuple = tuples[i]; + var view = viewPort.remove(tuple.record.previousIndex); + if (isPresent(tuple.record.currentIndex)) { + tuple.view = view; + ListWrapper.push(movedTuples, tuple); + } + } + return movedTuples; + } + + static bulkInsert(tuples, viewPort) { + tuples.sort((a, b) => a.record.currentIndex - b.record.currentIndex); + for (var i = 0; i < tuples.length; i++) { + var tuple = tuples[i]; + if (isPresent(tuple.view)) { + viewPort.insert(tuple.view, tuple.record.currentIndex); + } else { + tuple.view = viewPort.create(tuple.record.currentIndex); + } + } + return tuples; + } +} + +class RecordViewTuple { + view: View; + record: any; + constructor(record, view) { + this.record = record; + this.view = view; + } +} diff --git a/modules/directives/test/ng_repeat_spec.js b/modules/directives/test/ng_repeat_spec.js new file mode 100644 index 0000000000..f71b1a8130 --- /dev/null +++ b/modules/directives/test/ng_repeat_spec.js @@ -0,0 +1,239 @@ +import {describe, xit, it, expect, beforeEach, ddescribe, iit} from 'test_lib/test_lib'; + +import {DOM} from 'facade/dom'; + +import {Injector} from 'di/di'; +import {ChangeDetector} from 'change_detection/change_detector'; +import {Parser} from 'change_detection/parser/parser'; +import {Lexer} from 'change_detection/parser/lexer'; + +import {Compiler, CompilerCache} from 'core/compiler/compiler'; +import {OnChange} from 'core/compiler/interfaces'; +import {DirectiveMetadataReader} from 'core/compiler/directive_metadata_reader'; + +import {Decorator, Component, Template} from 'core/annotations/annotations'; +import {TemplateConfig} from 'core/annotations/template_config'; + +import {ViewPort} from 'core/compiler/viewport'; +import {MapWrapper, ListWrapper} from 'facade/collection'; +import {NgRepeat} from 'directives/ng_repeat'; + +export function main() { + describe('ng-repeat', () => { + var view, cd, compiler, component; + beforeEach(() => { + compiler = new Compiler(null, new DirectiveMetadataReader(), new Parser(new Lexer()), new CompilerCache()); + }); + + function createElement(html) { + return DOM.createTemplate(html).content.firstChild; + } + + function createView(pv) { + component = new TestComponent(); + view = pv.instantiate(null); + view.hydrate(new Injector([]), null, component); + cd = new ChangeDetector(view.recordRange); + } + + function compileWithTemplate(template) { + return compiler.compile(TestComponent, createElement(template)); + } + + var TEMPLATE = '
{{item.toString()}};
'; + + it('should reflect initial elements', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('1;2;'); + done(); + }); + }); + + it('should reflect added elements', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + cd.detectChanges(); + + ListWrapper.push(component.items, 3); + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('1;2;3;'); + done(); + }); + }); + + it('should reflect removed elements', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + cd.detectChanges(); + + ListWrapper.removeAt(component.items, 1); + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('1;'); + done(); + }); + }); + + it('should reflect moved elements', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + cd.detectChanges(); + + ListWrapper.removeAt(component.items, 0); + ListWrapper.push(component.items, 1); + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('2;1;'); + done(); + }); + }); + + it('should reflect a mix of all changes (additions/removals/moves)', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + component.items = [0, 1, 2, 3, 4, 5]; + cd.detectChanges(); + + component.items = [6, 2, 7, 0, 4, 8]; + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('6;2;7;0;4;8;'); + done(); + }); + }); + + it('should iterate over an array of objects', () => { + compileWithTemplate('').then((pv) => { + createView(pv); + + // INIT + component.items = [{'name': 'misko'}, {'name':'shyam'}]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('misko;shyam;'); + + // GROW + ListWrapper.push(component.items, {'name': 'adam'}); + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('misko;shyam;adam;'); + + // SHRINK + ListWrapper.removeAt(component.items, 2); + ListWrapper.removeAt(component.items, 0); + cd.detectChanges(); + + expect(DOM.getText(view.nodes[0])).toEqual('shyam;'); + }); + }); + + it('should gracefully handle nulls', (done) => { + compileWithTemplate('').then((pv) => { + createView(pv); + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual(''); + done(); + }); + }); + + it('should gracefully handle ref changing to null and back', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('1;2;'); + + component.items = null; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual(''); + + component.items = [1, 2, 3]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('1;2;3;'); + done(); + }); + }); + + it('should throw on ref changing to string', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('1;2;'); + + component.items = 'whaaa'; + expect(() => cd.detectChanges()).toThrowError(); + done(); + }); + }); + + it('should works with duplicates', (done) => { + compileWithTemplate(TEMPLATE).then((pv) => { + createView(pv); + var a = new Foo(); + component.items = [a, a]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('foo;foo;'); + done(); + }); + }); + +/* +TODO(rado): enable after compiler is fixed. + it('should repeat over nested arrays', (done) => { + compileWithTemplate( + '' + ).then((pv) => { + createView(pv); + component.items = [['a', 'b'], ['c','d']]; + cd.detectChanges(); + cd.detectChanges(); + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual(''); + done(); + }); + }); + +TODO(rado): enable after compiler is fixed. + it('should display indices correctly', (done) => { + var INDEX_TEMPLATE = '
{{index.toString()}};
'; + compileWithTemplate(INDEX_TEMPLATE).then((pv) => { + createView(pv); + component.items = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('0123456789'); + + component.items = [1, 2, 6, 7, 4, 3, 5, 8, 9, 0]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('0123456789'); + done(); + }); + }); +*/ + }); +} + +class Foo { + toString() { + return 'foo'; + } +} + +@Component({ + selector: 'test-cmp', + template: new TemplateConfig({ + inline: '', // each test swaps with a custom template. + directives: [NgRepeat] + }) +}) +class TestComponent { + items: any; + item: any; + constructor() { + this.items = [1, 2]; + } +} diff --git a/modules/facade/src/dom.dart b/modules/facade/src/dom.dart index f01f55c656..9e25dadc4e 100644 --- a/modules/facade/src/dom.dart +++ b/modules/facade/src/dom.dart @@ -53,6 +53,9 @@ class DOM { static insertAfter(el, node) { el.parentNode.insertBefore(node, el.nextNode); } + static getText(Element el) { + return el.text; + } static setText(Text text, String value) { text.text = value; } diff --git a/modules/facade/src/dom.es6 b/modules/facade/src/dom.es6 index 3a95395d0c..f1013f2222 100644 --- a/modules/facade/src/dom.es6 +++ b/modules/facade/src/dom.es6 @@ -49,6 +49,9 @@ export class DOM { static setInnerHTML(el, value) { el.innerHTML = value; } + static getText(el: Element) { + return el.textContent; + } static setText(text:Text, value:string) { text.nodeValue = value; } diff --git a/test-main.js b/test-main.js index 438c6a4ca1..b558f319a0 100644 --- a/test-main.js +++ b/test-main.js @@ -24,6 +24,9 @@ System.paths = { 'di/*': './di/src/*.js', 'di/test/*': './di/test/*.js', + 'directives/*': './directives/src/*.js', + 'directives/test/*': './directives/test/*.js', + 'reflection/*': './reflection/src/*.js', 'reflection/test/*': './reflection/test/*.js', diff --git a/tools/build/runtime_paths.js b/tools/build/runtime_paths.js index 72a57fcd1b..92101fdd3e 100644 --- a/tools/build/runtime_paths.js +++ b/tools/build/runtime_paths.js @@ -3,6 +3,7 @@ System.paths = { 'change_detection/*': '/change_detection/lib/*.js', 'facade/*': '/facade/lib/*.js', 'di/*': '/di/lib/*.js', + 'directives/*': '/directives/lib/*.js', 'rtts_assert/*': '/rtts_assert/lib/*.js', 'test_lib/*': '/test_lib/lib/*.js', 'reflection/*': '/reflection/lib/*.js', @@ -11,4 +12,4 @@ System.paths = { 'benchmarks/*': '/benchmarks/web/*.js', 'benchmarks_external/*': '/benchmarks_external/web/*.js', }; -register(System); \ No newline at end of file +register(System);