Skip to content

Commit

Permalink
feat: change template micro-syntax to new syntax
Browse files Browse the repository at this point in the history
Old syntax:
- ng-repeat: #item in items;
- ng-repeat: #item; in: items;
- <template let-ng-repeat=“item” [in]=items>

New syntax:
- ng-repeat: var item in items;
- ng-repeat: var item; in items
- <template ng-repeat var-item [in]=items>


Notice that the var is now a standalone binding 
rather then an argument to ng-repeat. This will 
make the var bindings consistent with the rest of 
the system.

Closes angular#482
  • Loading branch information
mhevery committed Jan 30, 2015
1 parent b1e76c5 commit 9db13be
Show file tree
Hide file tree
Showing 14 changed files with 88 additions and 30 deletions.
4 changes: 3 additions & 1 deletion modules/change_detection/src/parser/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,12 @@ export class ASTWithSource extends AST {

export class TemplateBinding {
key:string;
keyIsVar:boolean;
name:string;
expression:ASTWithSource;
constructor(key:string, name:string, expression:ASTWithSource) {
constructor(key:string, keyIsVar:boolean, name:string, expression:ASTWithSource) {
this.key = key;
this.keyIsVar = keyIsVar;
// only either name or expression will be filled.
this.name = name;
this.expression = expression;
Expand Down
5 changes: 5 additions & 0 deletions modules/change_detection/src/parser/lexer.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ export class Token {
return (this.type == TOKEN_TYPE_KEYWORD);
}

isKeywordVar():boolean {
return (this.type == TOKEN_TYPE_KEYWORD && this._strValue == "var");
}

isKeywordNull():boolean {
return (this.type == TOKEN_TYPE_KEYWORD && this._strValue == "null");
}
Expand Down Expand Up @@ -469,6 +473,7 @@ var OPERATORS = SetWrapper.createFromList([


var KEYWORDS = SetWrapper.createFromList([
'var',
'null',
'undefined',
'true',
Expand Down
26 changes: 22 additions & 4 deletions modules/change_detection/src/parser/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,19 @@ class _ParseAST {
}
}

optionalKeywordVar():boolean {
if (this.peekKeywordVar()) {
this.advance();
return true;
} else {
return false;
}
}

peekKeywordVar():boolean {
return this.next.isKeywordVar() || this.next.isOperator('#');
}

expectCharacter(code:int) {
if (this.optionalCharacter(code)) return;
this.error(`Missing expected ${StringWrapper.fromCharCode(code)}`);
Expand Down Expand Up @@ -469,21 +482,26 @@ class _ParseAST {
parseTemplateBindings() {
var bindings = [];
while (this.index < this.tokens.length) {
var keyIsVar:boolean = this.optionalKeywordVar();
var key = this.expectTemplateBindingKey();
this.optionalCharacter($COLON);
var name = null;
var expression = null;
if (this.next !== EOF) {
if (this.optionalOperator("#")) {
name = this.expectIdentifierOrKeyword();
} else {
if (keyIsVar) {
if (this.optionalOperator("=")) {
name = this.expectTemplateBindingKey();
} else {
name = '\$implicit';
}
} else if (!this.peekKeywordVar()) {
var start = this.inputIndex;
var ast = this.parseExpression();
var source = this.input.substring(start, this.inputIndex);
expression = new ASTWithSource(ast, source, this.location);
}
}
ListWrapper.push(bindings, new TemplateBinding(key, name, expression));
ListWrapper.push(bindings, new TemplateBinding(key, keyIsVar, name, expression));
if (!this.optionalCharacter($SEMICOLON)) {
this.optionalCharacter($COMMA);
};
Expand Down
34 changes: 31 additions & 3 deletions modules/change_detection/test/parser/parser_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,16 @@ export function main() {
return ListWrapper.map(templateBindings, (binding) => binding.key );
}

function keyValues(templateBindings) {
return ListWrapper.map(templateBindings, (binding) => {
if (binding.keyIsVar) {
return '#' + binding.key + (isBlank(binding.name) ? '' : '=' + binding.name);
} else {
return binding.key + (isBlank(binding.expression) ? '' : `=${binding.expression}`)
}
});
}

function names(templateBindings) {
return ListWrapper.map(templateBindings, (binding) => binding.name );
}
Expand Down Expand Up @@ -466,9 +476,7 @@ export function main() {

it('should detect names as value', () => {
var bindings = parseTemplateBindings("a:#b");
expect(names(bindings)).toEqual(['b']);
expect(exprSources(bindings)).toEqual([null]);
expect(exprAsts(bindings)).toEqual([null]);
expect(keyValues(bindings)).toEqual(['a', '#b']);
});

it('should allow space and colon as separators', () => {
Expand Down Expand Up @@ -497,6 +505,26 @@ export function main() {
var bindings = parseTemplateBindings("a 1,b 2", 'location');
expect(bindings[0].expression.location).toEqual('location');
});

it('should support var/# notation', () => {
var bindings = parseTemplateBindings("var i");
expect(keyValues(bindings)).toEqual(['#i']);

bindings = parseTemplateBindings("#i");
expect(keyValues(bindings)).toEqual(['#i']);

bindings = parseTemplateBindings("var i-a = k-a");
expect(keyValues(bindings)).toEqual(['#i-a=k-a']);

bindings = parseTemplateBindings("keyword var item; var i = k");
expect(keyValues(bindings)).toEqual(['keyword', '#item=\$implicit', '#i=k']);

bindings = parseTemplateBindings("keyword: #item; #i = k");
expect(keyValues(bindings)).toEqual(['keyword', '#item=\$implicit', '#i=k']);

bindings = parseTemplateBindings("directive: var item in expr; var a = b", 'location');
expect(keyValues(bindings)).toEqual(['directive', '#item=\$implicit', 'in=expr in location', '#a=b']);
});
});

describe('parseInterpolation', () => {
Expand Down
8 changes: 6 additions & 2 deletions modules/core/src/compiler/pipeline/compile_element.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export class CompileElement {
textNodeBindings:Map;
propertyBindings:Map;
eventBindings:Map;

/// Store directive name to template name mapping.
/// Directive name is what the directive exports the variable as
/// Template name is how it is reffered to it in template
variableBindings:Map;
decoratorDirectives:List<DirectiveMetadata>;
templateDirective:DirectiveMetadata;
Expand Down Expand Up @@ -102,11 +106,11 @@ export class CompileElement {
MapWrapper.set(this.propertyBindings, property, expression);
}

addVariableBinding(contextName:string, templateName:string) {
addVariableBinding(directiveName:string, templateName:string) {
if (isBlank(this.variableBindings)) {
this.variableBindings = MapWrapper.create();
}
MapWrapper.set(this.variableBindings, contextName, templateName);
MapWrapper.set(this.variableBindings, templateName, directiveName);
}

addEventBinding(eventName:string, expression:AST) {
Expand Down
4 changes: 2 additions & 2 deletions modules/core/src/compiler/pipeline/element_binder_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ export class ElementBinderBuilder extends CompileStep {
MapWrapper.get(compileElement.propertyBindings, elProp) :
null;
if (isBlank(expression)) {
throw new BaseException('No element binding found for property '+elProp
+' which is required by directive '+stringify(directive.type));
throw new BaseException("No element binding found for property '" + elProp
+ "' which is required by directive '" + stringify(directive.type) + "'");
}
var len = dirProp.length;
var dirBindingName = dirProp;
Expand Down
4 changes: 2 additions & 2 deletions modules/core/src/compiler/pipeline/property_binding_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {CompileElement} from './compile_element';
import {CompileControl} from './compile_control';

// TODO(tbosch): Cannot make this const/final right now because of the transpiler...
var BIND_NAME_REGEXP = RegExpWrapper.create('^(?:(?:(bind)|(let)|(on))-(.+))|\\[([^\\]]+)\\]|\\(([^\\)]+)\\)');
var BIND_NAME_REGEXP = RegExpWrapper.create('^(?:(?:(bind)|(var)|(on))-(.+))|\\[([^\\]]+)\\]|\\(([^\\)]+)\\)');

/**
* Parses the property bindings on a single element.
Expand Down Expand Up @@ -40,7 +40,7 @@ export class PropertyBindingParser extends CompileStep {
// Note: We assume that the ViewSplitter already did its work, i.e. template directive should
// only be present on <template> elements any more!
if (!(current.element instanceof TemplateElement)) {
throw new BaseException('let-* is only allowed on <template> elements!');
throw new BaseException('var-* is only allowed on <template> elements!');
}
current.addVariableBinding(bindParts[4], attrValue);
} else if (isPresent(bindParts[3])) {
Expand Down
2 changes: 1 addition & 1 deletion modules/core/src/compiler/pipeline/view_splitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class ViewSplitter extends CompileStep {
var bindings = this._parser.parseTemplateBindings(templateBindings, this._compilationUnit);
for (var i=0; i<bindings.length; i++) {
var binding = bindings[i];
if (isPresent(binding.name)) {
if (binding.keyIsVar) {
compileElement.addVariableBinding(binding.key, binding.name);
} else if (isPresent(binding.expression)) {
compileElement.addPropertyBinding(binding.key, binding.expression);
Expand Down
6 changes: 3 additions & 3 deletions modules/core/test/compiler/integration_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export function main() {
});

it('should support template directives via `<template>` elements.', (done) => {
compiler.compile(MyComp, el('<div><template let-some-tmpl="greeting"><copy-me>{{greeting}}</copy-me></template></div>')).then((pv) => {
compiler.compile(MyComp, el('<div><template some-tmplate var-greeting="some-tmpl"><copy-me>{{greeting}}</copy-me></template></div>')).then((pv) => {
createView(pv);

cd.detectChanges();
Expand All @@ -112,7 +112,7 @@ export function main() {
});

it('should support template directives via `template` attribute.', (done) => {
compiler.compile(MyComp, el('<div><copy-me template="some-tmpl #greeting">{{greeting}}</copy-me></div>')).then((pv) => {
compiler.compile(MyComp, el('<div><copy-me template="some-tmplate: var greeting=some-tmpl">{{greeting}}</copy-me></div>')).then((pv) => {
createView(pv);

cd.detectChanges();
Expand Down Expand Up @@ -170,7 +170,7 @@ class ChildComp {
}

@Template({
selector: '[some-tmpl]'
selector: '[some-tmplate]'
})
class SomeTemplate {
constructor(viewPort: ViewPort) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ export function main() {
var pipeline = createPipeline({propertyBindings: MapWrapper.create(), directives: [SomeDecoratorDirectiveWithBinding]});
expect( () => {
pipeline.process(el('<div viewroot prop-binding directives>'));
}).toThrowError('No element binding found for property boundprop1 which is required by directive SomeDecoratorDirectiveWithBinding');
}).toThrowError("No element binding found for property 'boundprop1' which is required by directive 'SomeDecoratorDirectiveWithBinding'");
});

});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ export function main() {
expect(MapWrapper.get(results[0].propertyBindings, 'a').source).toEqual('{{b}}');
});

it('should detect let- syntax', () => {
var results = createPipeline().process(el('<template let-a="b"></template>'));
expect(MapWrapper.get(results[0].variableBindings, 'a')).toEqual('b');
it('should detect var- syntax', () => {
var results = createPipeline().process(el('<template var-a="b"></template>'));
expect(MapWrapper.get(results[0].variableBindings, 'b')).toEqual('a');
});

it('should not allow let- syntax on non template elements', () => {
it('should not allow var- syntax on non template elements', () => {
expect( () => {
createPipeline().process(el('<div let-a="b"></div>'))
}).toThrowError('let-* is only allowed on <template> elements!');
createPipeline().process(el('<div var-a="b"></div>'))
}).toThrowError('var-* is only allowed on <template> elements!');
});

it('should detect () syntax', () => {
Expand Down
6 changes: 3 additions & 3 deletions modules/core/test/compiler/pipeline/view_splitter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ export function main() {
});

it('should add variable mappings from the template attribute', () => {
var rootElement = el('<div><div template="varName #mapName"></div></div>');
var rootElement = el('<div><div template="var varName=mapName"></div></div>');
var results = createPipeline().process(rootElement);
expect(results[1].variableBindings).toEqual(MapWrapper.createFromStringMap({'varName': 'mapName'}));
expect(results[1].variableBindings).toEqual(MapWrapper.createFromStringMap({'mapName': 'varName'}));
});

it('should add entries without value as attribute to the element', () => {
Expand All @@ -101,4 +101,4 @@ export function main() {
});

});
}
}
2 changes: 1 addition & 1 deletion modules/directives/src/ng_repeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class NgRepeat extends OnChange {
}

perViewChange(view, record) {
view.setLocal('ng-repeat', record.item);
view.setLocal('\$implicit', record.item);
view.setLocal('index', record.currentIndex);
}

Expand Down
3 changes: 2 additions & 1 deletion modules/directives/test/ng_repeat_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ export function main() {


it('should display indices correctly', (done) => {
var INDEX_TEMPLATE = '<div><copy-me template="ng-repeat #item in items index #i">{{i.toString()}}</copy-me></div>';
var INDEX_TEMPLATE =
'<div><copy-me template="ng-repeat: var item in items; var i=index">{{i.toString()}}</copy-me></div>';
compileWithTemplate(INDEX_TEMPLATE).then((pv) => {
createView(pv);
component.items = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
Expand Down

0 comments on commit 9db13be

Please sign in to comment.