Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with ArrayBuffer #299

Merged
merged 1 commit into from Apr 26, 2016

Conversation

@nickuraltsev
Copy link
Member

commented Apr 18, 2016

I think we should not wrap an ArrayBuffer in a DataView. XMLHttpRequest#send(ArrayBuffer) is deprecated, but XMLHttpRequest#send(ArrayBufferView) does not seem to be supported by some browsers like IE 11. BTW, fetch sends an ArrayBuffer as is, so my suggestion is to do the same.

This PR fixes #268.

@coveralls

This comment has been minimized.

Copy link

commented Apr 18, 2016

Coverage Status

Coverage decreased (-0.05%) to 91.317% when pulling 4d83ba9 on nickuraltsev:array-buffer-fix into 5c58b83 on mzabriskie:master.

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Apr 21, 2016

What's the potential fall out for using ArrayBuffer if it is deprecated? Seems like this will work short term, but at some point will cause problems.

Note: Be aware to stop using a plain ArrayBuffer as parameter. This is not part of the XMLHttpRequest specification any longer. Use an ArrayBufferView instead (see compatibility table for version information).

@nickuraltsev

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2016

@mzabriskie Yes, I saw that. The problem is that the deprecated API seems to have a better browser support than the new API (see below). This PR actually allows users to choose what to use - ArrayBuffer or ArrayBufferView (instead of forcing them to use ArrayBufferView which does not seem to be supported by IE).

arraybuffer

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

So in the case of wanting to use ArrayBufferView they would have to pass that in explicitly, instead of us converting ArrayBuffer for them, as is changed in this PR?

@nickuraltsev

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2016

Right. They will be able to choose between ArrayBuffer and ArrayBufferView as if they were using XMLHttpRequest directly.

@mzabriskie mzabriskie merged commit aeac3e1 into axios:master Apr 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.