Open Aigeec opened 6 years ago
Great idea! I didn't understand the use case before, but now that you've described it I do.
One thing that reviewing the tests made me realize is that this approach couples Oboe to the FormData
global. From what I can tell, this is available in browsers, but not in Node. If the use case is to support data sources that require a file upload to start, then we are adding this feature only in the browser. Perhaps in Node it's already possible to do this by making the request externally, but I think it would be good to aim for feature parity anyway. My reasoning is that less differences between different "versions" of Oboe will make it easier to reason about the API.
What do you think of those concerns @Aigeec ?
Yes I understand the concern. So the requirement is to maintain feature parity and the same api.
So the source of concern is around body handling, specifically defaults.js#L11-L23. I think there are some issues with that body handling being in shared code regardless as adding a content-length header is invalid for a browser (#146) but required for node (#141).
We could have different implementations as there is for the http-streaming or just move that code in there.
I'm not sure exactly how we would do type checking on the node side so we'd have to go with some kind of duck typing approach to determine if the body was in fact FormData of some description. But that should be doable.
I'm very interested in using this feature, would you say it's ready for use on the browser side @Aigeec? I need to parse a streamed JSON response from an endpoint that expects a file and was bitten by OboeJS lack of file upload support at the very last moment.
resolves #70
Will now send formData such as a file as the request body