oboe.js icon indicating copy to clipboard operation
oboe.js copied to clipboard

Feature/use fetch api

Open Aigeec opened this issue 7 years ago • 7 comments

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.

Aigeec avatar Mar 25 '18 00:03 Aigeec

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

JuanCaicedo avatar Mar 27 '18 16:03 JuanCaicedo

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.

Aigeec avatar Mar 27 '18 19:03 Aigeec

Ok, then this proposal sounds good to me!

JuanCaicedo avatar Mar 28 '18 17:03 JuanCaicedo

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.

Aigeec avatar Mar 29 '18 20:03 Aigeec

@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.

jasonleehodges avatar Jul 30 '19 22:07 jasonleehodges

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

Aigeec avatar Oct 12 '19 13:10 Aigeec

@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!

JuanCaicedo avatar Oct 15 '19 14:10 JuanCaicedo