data-bind parsing issue when filtering using hash index lookup syntax #517

Open
odorcicd opened this Issue Aug 9, 2012 · 3 comments

Projects

None yet

4 participants

@odorcicd
odorcicd commented Aug 9, 2012
hash = new Batman.SimpleHash
  'a': 1
  'b': 2

<select>
<option data-foreach-key="hash" data-bind="'Number: ' | append hash[key]"></option>
</select>

I'd expect each option element to show Number: 1, but it comes back blank.

Using a data-context works however:

<select>
<option data-foreach-key="hash" data-context-number="hash[key]" data-bind="'Number: ' | append number"></option>
</select>
@airhorns
Collaborator

Operator precedence? More like operator pestilence!

Seriously though, the issue here is that the filter syntax is purposefully less than powerful in the attempt to force people to rethink complicated filter chains and hopefully move them to an accessor or function on the view. This will be greatly helped by moving the filter lookup chain to be the same as the key look up chain, so you can make locally scoped filters for particular views.

The question is does introducing operator precedence (and the associated code complexity) move us forward or backward? Evaluating the much more complex filter chain parsing stage I assume we'd need, the complicated filter strings that would be come doable, and the inevitable slippery slope argument (well its not useful unless we add brackets!) I give a final -1. @nciagra @jamesmacaulay what do you guys think?

Also, @odorcicd you can accomplish what you were trying to do by switching to prepend:

<option data-foreach-key="hash" data-bind="hash[key] | prepend 'Number: '"></option>

This works because the [key] piece of the filter string here gets implicitly translated to a get filter call, and the filters are then run in order left to right. So, your original filter string expands to this:

<option data-bind="'Number: ' | append hash | get key"></option>

which tries to do a .get('...') on the result of a string plus a Hash. Realizing the problem, we can move the get to be on the proper object, and then add the string second:

<option data-foreach-key="hash" data-bind="hash | get key | prepend 'Number: '"></option>

which condenses to the solution above using the shorthand get syntax. What can do about this problem? @odorcicd do you think your issue arose only because of the [] syntax seemingly being more important?

@nickjs
Member
nickjs commented Sep 19, 2012

I do think the [] should work consistently regardless of where/which filter it's being used in conjunction with. However, beyond that, any added complexity to filter lookups is absolutely not what we want to do. The line is thus: filters should only be used for presentation and/or referencing nodes from CoffeeScript land and vice versa. Any type of actual logic or computations should be done in an accessor, and therefore in CoffeeScript land.

Denis, in this case you are trying to put logic in your filters. Boohoo to you.

@jamesmacaulay
Collaborator

@nciagra sez:

Any type of actual logic or computations should be done in an accessor, and therefore in CoffeeScript land.

What "logic" is @odorcicd putting in his filter, exactly? If there's too much logic in this filter, then there's too much logic in 99% of the batman filters out there (and certainly all filters perform "computations"). He's using append in a completely straightforward way which happens to be broken.

I basically agree that "any added complexity to filter lookups is absolutely not what we want to do." However, the syntax here is just deceptive, plain and simple. We should all be on the same page that the complexity of fixing it lies in the framework implementation, not in the usability of the syntax.

That said, I think it should be a much higher priority to allow the easy controller- and view-specific filter definitions that @hornairs mentioned, by unifying the context lookups. Then perhaps it would be fair to suggest that @odorcicd should write a custom filter just for this occasion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment