yoshuawuyts / barracks

:mountain_railway: action dispatcher for unidirectional data flows
MIT License
177 stars 22 forks source link

Wrap handlers #59

Closed yoshuawuyts closed 8 years ago

yoshuawuyts commented 8 years ago

Adds the functionality described in https://github.com/yoshuawuyts/choo/issues/145. Builds on #58. Pretttttty excited about this :sparkles:

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.5%) to 95.152% when pulling 7e17e8269b3b7a7a1c4a2bc9c35099f4f29e654b on wrap-handlers into b381a971ec72102caf6570756bb3d63a8e6f885e on master.

myobie commented 8 years ago

I am very excited about this. I already created an effectWrapper function that returns a wrapped function so I can use another promise based storage api more easily in the effects, but I have to use it repeatedly in my model.

An example I have is:

export function wrapEffect (cb) {
  return async (data, state, send, done) => {
    const newSend = (action, data = null) => {
      return new Promise((resolve, reject) => {
        send(name, data, (err, res) => {
          if (err) {
            done(err) // communicate error to choo
            reject(err) // throw
          } else {
            resolve(res) // move forward
          }
        })
      })
    }
    await cb(data, state, newSend)
    done() // auto-done when everything is done
  }
}

which allowed me to write this in the end:

app.model({
  effects: {
    reloadMailsCount: wrapEffect(async (data, state, send) => {
      const count = await storage.getMailsCount()
      await send('receiveMailsCount', count)
    })
  }
})

Is this the type of thing you are hoping to enable?

yoshuawuyts commented 8 years ago

@myobie Hah yeah, pretty much! I'm a strong opponent of Promises myself, but I want people that like them to be able to use them as they please (as with other paradigms).

A little note: for your implementation to be correct you'd probably have to wrap it in a try ... catch so any errors that bubble up can be caught for the send(err) call. Also probably cool to allow it to return a value synchronously which would be passed into send(null, value) :sparkles:

myobie commented 8 years ago

@yoshuawuyts great idea, I will do that. And yes, promises + try/catch do sometimes make my head hurt.

I'm not entirely sure what send(null, value) would do, so I will do some research there.

yoshuawuyts commented 8 years ago

@myobie so when calling send(name, data, callback(err, res)) the receiving method can trigger the callback by calling done(err, res). It's basically delegation of control, and once it's done it either returns an error or data (where data can be null / undefined). This essentially creates a CSP style system. This is not explicitly mentioned anywhere though (yay for docs) but figured I'd mention it here so that you can at least have a good time implementing a package in this fashion (':

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.5%) to 97.972% when pulling e6c28e0d459ba543b747e315a9a4c2d539c24de6 on wrap-handlers into 7756160567e69e2c42880424a3021bdf12edb078 on master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.2%) to 98.586% when pulling e72f758f9ae5edd4cf69435daf7a0803be44ce44 on wrap-handlers into 7756160567e69e2c42880424a3021bdf12edb078 on master.