component / reactive

Tiny reactive template engine
382 stars 48 forks source link

remove .use in favor of specifying plugins in config #126

Closed defunctzombie closed 10 years ago

defunctzombie commented 10 years ago

.use doesn't work because we have no way to pass the plugin bindings like "each" where you might want to use your custom bindings inside of the loop within each model context. The way it is handled right now is incorrect because the model context is not what you would expect for a custom binding inside of an each.

The main reason I don't like .use is because it separates potentially important bindings/pieces from the "construction" phase. Since we don't have a "render" phase, reactive isn't really setup to properly support not knowing certain pieces upfront when you build the instance.

I am thinking the syntax will change from

var view = reactive(template, model, options);
view.use(...);

to

var view = reactive(template, model, {
    bindings: {
         'data-whatever', fn
         'moment': fn
    }
});

or possibly

var view = reactive(template, model, {
    plugins: [plugin_fn, plugin_fn2]
});

Right now I am leaning towards the bindings approach simply because we don't have a generic needs for "plugins" at this time. It also means that the consumer of a custom binding function can decide what the binding will be called in their template versus having to hunt for the name in a plugin. Yes, it looks a bit more verbose and "configuration" like over the .use but I find it more explicit.

Again, the main reason to move into the config object is because of how "each" bindings work and them needing the plugins from the parent. Without knowing those ahead of time, other things become more complicated unless we add an explicit "render" step.

ericgj commented 10 years ago

:thumbsup:

defunctzombie commented 10 years ago

This landed in 1.1.0

anthonyshort commented 10 years ago

I think this might have been a bad move. You could have just kept all of the bindings inside of another object and passed that around. Then the reactive instance either accepts bindings in the options or creates a blank set of bindings.

Then you just grab all of those bindings and pass them to the child reactive so it can render the same way.

defunctzombie commented 10 years ago

Not possible if you want to use the bindings in an "each" and have some data to start because of how we immediately render. If you can come up with a clever way to do it I would love to see it, but I couldn't think of a non-trivial way.

The benefit of this approach is that it also let's you pick the name of the binding which does have pros and con's. On Apr 3, 2014 7:10 PM, "Anthony Short" notifications@github.com wrote:

I think this might have been a bad move. You could have just kept all of the bindings inside of another object and passed that around. Then the reactive instance either accepts bindings in the options or creates a blank set of bindings.

Then you just grab all of those bindings and pass them to the child reactive so it can render the same way.

Reply to this email directly or view it on GitHubhttps://github.com/component/reactive/issues/126#issuecomment-39516162 .

anthonyshort commented 10 years ago

Yeah this is how I did it with ripple:

https://github.com/ripplejs/ripple/blob/master/lib/bindings.js The bindings object. I originally called this the compiler. Basically renders a view against a set of bindings.

https://github.com/ripplejs/ripple/blob/master/lib/view.js#L64 This is the view. You can pass in a bindings object or one is created

https://github.com/ripplejs/ripple/blob/master/lib/view.js#L248 Render is called

https://github.com/ripplejs/ripple/blob/master/lib/render.js This is where the bindings and rendered to the view.

Works pretty well. I can do iteration like this and pass down all the bindings to sub-views. This allows sub-views to essentially be completely independent, but they have the same "engine" behind the scenes for rendering.

anthonyshort commented 10 years ago

https://github.com/ripplejs/each/blob/master/index.js#L47

There it is in practice.