AmpersandJS / ampersand-state

Core state management object.
MIT License
141 stars 75 forks source link

Pass change event options from children to parent #251

Open valentin-nemcev opened 7 years ago

valentin-nemcev commented 7 years ago

I use event options to pass metadata when setting properties, this change fixes a bug when children change event options are not passed to parent event listeners

dhritzkiv commented 7 years ago

This makes sense to me. I'm going to think about this one for a bit.

Could you share some use cases for this? I'm having trouble finding the need to know the options on a change event listener.

Tests seem to pass (despite the saucelabs credentials issue). This would be a semver major change.

valentin-nemcev commented 7 years ago

I called it a bug because I use it for regular set method and totally expected it to work across children-parent boundary, and this behavior is hard to override in client code.

My general use case is selectively skipping event listeners, mostly to break complicated loops in bidirectional bindings. Simplified example is All checkbox:

All depends on items and items depend on all. In this example you could easily make sure that everything is consistent and no stackoverflow happens when user clicks on checkbox, but sometimes its hard or undesirable because of performance (like when the loop involves a request to backend), or you don't need full consistency. In that case you want to manually break the loop by writing something like this:

// this.allChecked depends on checkbox values
this.listenTo(this, 'change:allChecked',  (model, value, {skipAllCheck} = {}) =>
    skipAllCheck || this.setCheckboxValues(value, {skipAllCheck: true}))

My real use case involved Paginator that is a child of Filters. Filters send their values to backend and backend response updates filter values (including paginator).