aurelia / templating-binding

An implementation of the templating engine's Binding Language abstraction which uses a pluggable command syntax.
MIT License
32 stars 26 forks source link

Support Enhanced Object Literals in bindings #75

Closed vbauerster closed 8 years ago

vbauerster commented 8 years ago

I've a working code like:

<compose model.bind="{ model: model, columnId: columnId }" view-model="widgets/${item.type}"></compose>

Next, my logical thought was to try the same, but with ES6 in mind:

<compose model.bind="{ model, columnId }" view-model="widgets/${item.type}"></compose>

However, I got the following error:

ERROR [app-router] Error: Parser Error: Missing expected : at column 8 in [{ model, columnId }]

It would be nice to support such binding notation, as in ES6 we ca do:

var model = 'model';
var obj = { model };
dev-zb commented 8 years ago

Only looked around the code a bit and put this together. Perhaps it's too hackish. :confused: https://github.com/z-brooks/binding/blob/master/src/parser.js#L353

This might not be sufficient as I'm not 100% on how to write tests but they all pass. https://github.com/z-brooks/binding/blob/master/test/parser.spec.js#L216

jdanyow commented 8 years ago

@z-brooks nice! Very clever reversing the index.

I don't think we can reuse the parseAccessOrCallScope method in this case, it's probably too permissive. For example, I think it would allow { foo.bar.baz } which is invalid right?

Let's determine which of the new ES6 object initializer syntaxes we'll support- probably only the first one here right?

syntax https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer

And then add explicit logic to the parser for this along with tests for the negative scenarios. Does that make sense?

dev-zb commented 8 years ago

Thanks! Yeah that makes sense. I didn't consider the other forms of shorthand; might have to give it some thought.

Invalid property names are the reason for the peek.key check. It seemed the lexer would only set the key property for a token when the identifier is valid. That's also the why I decided on parseAccessOrCallScope. Let me know if I'm wrong there.

I wasn't sure of the best way to check for the negative or failure, but I made up this test: https://github.com/z-brooks/binding/blob/master/test/parser.spec.js#L228

Trying to parse { foo.bar } or { "foo" } using first test will fail when the parser throws from the expect method and the failure test will pass for the same reason.

jdanyow commented 8 years ago

makes sense, all this looks good- would you mind rebasing on master and sending a PR with these changes? If you haven't signed the CLA, please do (details in contributing.md).

Much appreciated!

jdanyow commented 8 years ago

closing- thanks @z-brooks