MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
14.01k stars 926 forks source link

Should `m.prop()` be observable? #1116

Closed dead-claudia closed 8 years ago

dead-claudia commented 8 years ago

I'm mostly neutral on it, but here's what I'm proposing: m.prop() should accept an optional second argument, a callback, that is called on each value's change with the new value. Something like this:

var prop = m.prop(undefined, function (value) {
    console.log("My new value is: " + JSON.stringify(value))
})

prop(0) // logs 'My new value is: 0'
prop("4 x 4 = 12") // logs 'My new value is: "4 x 4 = 12"'

I know a few Mithril users would appreciate that in core, and it's trivial to add. Just inviting a little discussion here to see if it should be implemented.


In case you're wondering, here's what changes would be made:

// Current version
module.exports = function(store) {
    return function() {
        if (arguments.length > 0) store = arguments[0]
        return store
    }
}

// Proposed
module.exports = function(store, callback) {
    return function() {
        if (arguments.length > 0) {
            store = arguments[0]
            if (typeof callback === "function") callback(store)
        }
        return store
    }
}
bruce-one commented 8 years ago

Personally, I'd be a 👎 on this.

For situations where this is useful, I'd suggest it's too often more useful to wrap the whole prop, and not just watch, eg:

const _prop = m.prop()
const prop = (...args) => {
    if(args.length) {
        const value = args[0]
        const result = _prop( typeof value === 'string' ? value : String(value) ) // just a stupid example of "doing something"
        console.log(`My new value is: ${JSON.stringify(result)}`)
        return result
        // Or maybe something conditional
        // const value = parseInt(args[0])
        // if(isNaN(value) return _prop()
        // return _prop(value)
    }
    return _prop()
}
prop(0)
prop("4 x 4 = 12")

Or people can just a simple diy utility, (basic eg:

m.watchableProp = (initialValue, cb = function() {}) => {
  const _prop = m.prop(initialValue)
  return function watchableProp(...args) {
    if(args.length) {
      const result = _prop(args[0])
      cb(result)
      return result
    } else {
      return _prop()
    }
  }
}

const prop = m.watchableProp(null, (value) => {
  console.log(`My new value is: ${value}`)
})
prop(0)
prop("4 x 4 = 12")

)

IMO it's more flexible and more powerful to manually just wrap the prop as is logical, without adding clutter to Mithril core. And it's going to set someone along the most practical path for getting the most out of Mithril (if that makes sense? (that's just my perspective))

Maybe it's worth having a library/wiki page of common patterns where people can be pointed, rather than adding the functionality into core? (It feels like either a: it starts down the slippery slope of keep adding more functionality into the core framework; or b: discouraging composition. -- hence I'd be concerned about people trying to abuse the "observable" functionality, rather than tackling their problem in a simpler way.)

(Just my perspective :-) )

lhorie commented 8 years ago

props are now streams and replace promises as the return value of m.request (i.e. promises are gone). props are now very similar to flyd streams but also have similar semantics to promises.

Streams cover the use case of observables via the same mechanism that covers promise chaining (via .map), and in addition, it provides mechanisms for combining streams and guarantees atomic updates in O(n), as well as mechanisms for error propagation and handling (like promises do). It also enables update-on-write semantics, which are generally more efficient than doing reactivity in the view (i.e. update-on-read semantics). Also, it's fantasy-land friendly.

And the rewrite branch is still smaller than 0.2.x

There's some preliminary docs here: https://github.com/lhorie/mithril.js/blob/rewrite/docs/prop.md