AmpersandJS / ampersand-view

A smart base view for Backbone apps, to make it easy to bind collections and properties to the DOM.
http://ampersandjs.com
MIT License
92 stars 39 forks source link

Make bindings extensible #178

Closed RickButler closed 8 years ago

RickButler commented 8 years ago

I realize that extending private methods is inadvisable, but I had overwritten the binding call in _initializeBindings for several projects prior #105.

Would there be opposition to storing ampersand-dom-bindings as an attribute of the view (I would be okay with it being private), instead scoped to the module as var bindings?

For example I have a copy of ampersand-dom-bindings with additional types defined in getBindingFunc. This would allow me to extend the base view with my custom implementation of ampersand-dom-bindings, instead of overwriting the private method _upsertBindings. It would also allow for easier use of different binding implementations, as long as they return a key-tree-store.

cdaringe commented 8 years ago

interesting. what sort of custom bindings have you made? would they be helpful to roll back into ampersand-dom-bindings instead?

RickButler commented 8 years ago

@cdaringe, they are a subset of the text/innerHtml binding types.

For example, instead of polluting my models with derived types to convert a prop to a specific date/monetary or other format, I use a binding types to achieve this globally.

I use custom bindings instead of custom dataTypes because I may use the original value in other derived properties, bindings, or computations.

I would like propose changes to ampersand-dom-bindings complimentary to this proposal.

cdaringe commented 8 years ago

instead of polluting my models with derived types to convert a prop to a specific date/monetary or other format, ...

I like that. It's a great idea.

I think we should definitely support the extensibility you need. While im not gung-ho against your original proposal, perhaps we could simply add hooks to dom-bindings to make it more composable? that way, there's only change required in one spot. i think it's a little clearer to extend dom-bindings too. the flow remains unchanged in &-view, and there's no logic to debug outside of dom-bindings. i think it'd be easier to reason about. what are your thoughts?

RickButler commented 8 years ago

@cdaringe it depends on how the changes to ampersand-dom-bindings are made, it may still require other changes to &-view.

Either allowing the new bindings to be passed in from the view, or making dom-bindings a class that can be extended instead of a function would both require changes to the view.

cdaringe commented 8 years ago

dom-bindings a class that can be extended instead of a function

just thinking out loud. i was thinking something similar to ^^. basically add .extend() method to dom-bindings. on app init, you extend to add your custom bindings. maybe im missing something, but I don't think that'd required an &-view update

RickButler commented 8 years ago

@cdaringe that would be treating dom-bindings as a singleton like ampersand-app? What if you overwrite one of the original bindings but don't want to use it in every view or you just don't want them available in every view? I'm fine with doing it that way, just thought I would bring up the possible objections.

cdaringe commented 8 years ago

yea, that'd be an issue. frankly, I personally wouldn't want to squash a dom-binding. i can see that really confusing the rest of the dev-team. i'd prefer instead to maybe namespace it? like myproduct/binding-name. again, i'm super on board with the need to make it extensible, and think you've got a great use case. i'm iffy on the requirement to convert what is otherwise a general utility function provider into utility provider instances. from my current thinking, i wouldn't ever think of creating an isolated lodash instance to get special utility functions. i'd definitely extend it (as peeps commonly do), but instancifying dom-bindings seems like convolution from my perspective. again, just an opinion :)

orenmizr commented 8 years ago

hi Richard. As a user of ampersand, I'm interested in your use case (perhaps i'll adopt that approach). you said: "For example, instead of polluting my models with derived types to convert a prop to a specific date/monetary or other format, I use a binding types to achieve this globally."

can you give me an example where that is preferred?

RickButler commented 8 years ago

I opened AmpersandJS/ampersand-dom-bindings#53 so that we can discuss changes specific to ampersand-dom-bindings there.

RickButler commented 8 years ago

@cdaringe, I would like to look at adding the ability to pass in bindings defined in the view, similar to ampersand-state dataTypes.

E.G. defined an attribute on your view called bindingTypes as a key -> function store, then call bindings with context, bindings, and bindingTypes.

cdaringe commented 8 years ago

@RickButler, if you want to modify it to do so, i'll support it so long as:

throw up a PR, and we can get it going. thanks for explaining all your stuff so clearly :)