component / reactive

Tiny reactive template engine
383 stars 48 forks source link

each should be able to handle array emitters #124

Closed Swatinem closed 10 years ago

Swatinem commented 10 years ago

I’m a big fan of https://github.com/MatthewMueller/array, which has events builtin for any kind of array modification. Supporting that seems a lot nicer than monkeypatching (and missing half of) the array mutator methods.

defunctzombie commented 10 years ago

The last commit about emitters won't be accepted. I specifically moved away from any sort of assumptions about objects and events and moved the logic into adapters. Maybe the adapters can be extended to support array related features. That I would be interested in.

The other commits look good.

Yes, the remaining array mutator methods need to be added, but that is not hard. This patching approach makes it very easy to use with built in arrays and makes code more idiomatic js friendly and simpler.

Swatinem commented 10 years ago

So to really work with an array emitter, I would have to override the internal each binding. How would I do that?

defunctzombie commented 10 years ago

Dunno :) I am thinking the options when creating a reactive instance could be read or passed to the each binding. That way the binding could query the adapter for how to bind to an array (or have a separate array adapter). This would also be relevant to Backbone collections.

/cc @airportyh

airportyh commented 10 years ago

Maybe add a method to model adapters: subscribeCollectionUpdate(collection, fn). But now there's inconsistency in that models are wrapped by the adapter but collections are not. I have a feeling it would be cleaner to make model in the adapter into a param instead of a property, i.e. subscribe(model, prop, fn) instead of subscribe(prop, fn).

vweevers commented 10 years ago

I tried out reactive today (it's great!) and wanted to support an evented collection (similar to the array emitter mentioned above) for each. It's not difficult to create a custom each binding, however you can't do:

reactive(tmpl, model, {
  bindings: {
    each: myCustomBinding
  }
})

.. because it will be overridden by the default binding. I think that should be the other way around.

And, if an adapter requires the use of a custom binding, my idea is:

MyAdapter.bindings = { each: myCustomBinding }

E.g., if reactive sees a bindings property on the adapter, it will use those. For maximum flexibility, the merge order could be (last one taking precedence):

Array adapters would be great, especially if you can use different data types (arrays, collections, streams) within a single view. But for now, the easier road, I propose fixing the order of precedence. And perhaps document the expected behavior of the each binding.