search

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/use fetch api #169

Open Aigeec opened 6 years ago

Aigeec commented 6 years ago

A proposal on how we might support using the fetch-api as requested in #74 .

No api changes. Should be backwards compatible, there are no broken tests. Current http streaming tests run with no failures using the fetch api.

Incomplete, unit tests would obviously need to be added and I'm sure there are plenty of scenarios missed.

Great improvement with regard to heap usage using fetch.

Look forward to your comments.

JuanCaicedo commented 6 years ago

Cool! The internals of fetch are pretty new to me, but it looks like this would work well.

One thought is that we're going to make fetch the default if it's supported, and default back to XMLHttpRequest if it isn't. If it's a better way of requesting data over all, this SGTM. Would there be a scenario where a user would still want the underlying request to be XMLHttpRequest? If so, we should add some configuration for it. If we don't anticipate users wanting to do that though, completely abstracting it away seems good

Aigeec commented 6 years ago

Honestly I don't know why you would use xhr over fetch (I'm sure someone will have a reason). If they really wanted to they could just window.fetch = undefined and then it will fall back to xhr. But like I said why would they.

JuanCaicedo commented 6 years ago

Ok, then this proposal sounds good to me!

Aigeec commented 6 years ago

There is another feature request #55 to be able to pass stream a ReadableStream. I would suggest doing this as part of adding fetch api support. We could then simply pass oboe fetches ReadableStream.

Arguable this could be all in v3 where we just support streams.

jasonleehodges commented 5 years ago

@alecmerdler @JuanCaicedo @Aigeec Any updates on merging in this PR? I'd like to use this to stream json data as it seems like a simple interface. But I don't wish to use the underlying XHR request.

Aigeec commented 4 years ago

Will add config for this and fix the typo. Any thing else required?

JuanCaicedo commented 4 years ago

@Aigeec To be honest, I'm not much involved in this project any more 😅 So I think it's up to you whether would like to merge this.

I'm glad to help out by publishing another version (I think only the original author and I have npm access), but I don't have many plans to do much here asides of that.

Thank you for all your contributions!