jimhigson / oboe.js

A streaming approach to JSON. Oboe.js speeds up web applications by providing parsed objects before the response completes.
http://jimhigson.github.io/oboe.js-website/index.html
Other
4.79k stars 208 forks source link

feature(instanceApi): proposal for adding support for Promises #168

Closed Aigeec closed 6 years ago

Aigeec commented 6 years ago

Proposal for adding Promise API support #75

then would be called as per done catch called as per fail

Both done and fail will still also be called.

This does not add any external dependencies. A user could provide their own libarary via oboe.Promise or rely on native promises. Similar approach taken in es6-promisify.

If no promise library is available calling then or catch will throw a (rather wordy) error so we only. This means that the error is only throw if someone tries to use then and catch. Another option could by just not to add the methods and let someone look up the docs.

Example:

oboe('some/json/url')
  .node('result.*', (node) => {
    // your code here
  }).then((result) => {
    console.log(result);
  }).catch((err) => {
    console.error(err)
  })

Will write tests if you think its a reasonable approach.

Aigeec commented 6 years ago

I'd like to take a little longer to think through what the implications of this would be and if we need to do anything else to really do promises well. Since we're increasing the surface area of where/how Oboe can be used, I want to be sure we're not introducing any unexpected behavior.

In reality all we are doing is offering the done and fail methods in a promise compatible way. I don't think it is really any more than that. It does not add it to node or any of the emitting parts of oboe. Just the methods that are called at the end of processing.

Do these all behave the same? .fail(callback) .on('fail', callback) .catch(callback)

Do these all behave the same? .done(callback) .on('done, callback) .then(callback)

In all cases the callback would be executed with the same parameters passed in. The only difference with .catch(callback) or .then(callback) is that you would be able to chain it with other promise calls.

Will other errors be passed to .catch? (in the library code, in the request, or in the user defined handlers)

Only errors that would have been passed to .fail(callback) would be passed to .catch(callback).

Aigeec commented 6 years ago

I think I see where you are coming from. You might be right that people would expect those errors to be caught by the .catch and I believe making that change would be breaking. It would probably be quite intrusive to go and only throw things when then/catch has been used.

What if oboe() had a promise method that returned a promise you could then/catch, probably would lead to the same expectation.

JuanCaicedo commented 6 years ago

Promsifying

If I follow, the second option you're suggestion is

oboe('some/json/url')
  .node('result.*', (node) => { })
  .toPromise()
  .then((result) => { })
  .catch((err) => { })

One thing that I like about that is that it keeps promisifying separate from the initializer function. Not sure if that's common practice in other libraries or not.

Promise chain

In both cases, what should happen if a user wires up the handlers differently? For example

oboe('some/json/url')
  .then((result) => { })
  .node('result.*', (node) => { })
  .catch((err) => { })

My expectation is that .node should be undefined, because once you start a promise chain you'll always be getting back promises, not the instance API.

Multiple done/then

What order we the callbacks of this code be called in?

oboe('some/json/url')
  .done((result) => { })
  .then((result) => { })

If the two were to behave exactly the same, my expectation would be for done to be called and finished before then starts.

However, I don't think they are (and maybe this is okay). The callbacks for both will be called at the same time, and anything you return from done will stay inside of there.

JuanCaicedo commented 6 years ago

I hope that doesn't seem long winded 😅 I haven't made a lot of library API decisions and want to be sure we've thought it all through. Thanks for all your thoughts!

Aigeec commented 6 years ago

Lets not support promises. Lets just stick to the stream apis in Node and the Browser and simply implement that api. No mixing of technologies. Oboe is a streaming library, that is what it is for. So we should stick to stream apis.

Or we could do something like the fetch api with its mix of streams and promises.

JuanCaicedo commented 6 years ago

Ok, I think you've put into words the feeling I had when you mention "mixing technologies".

For people that really want to do it, I found there's someone who tried to a wrapper https://github.com/kinday/oboe-promise/blob/master/index.js.

It also shouldn't be hard to make a new promise with a promise constructor. Maybe what we could do is make a stackoverflow question and answer about it. That should probably rank highly on google if they search for "oboe promises"