whatwg / xhr

XMLHttpRequest Standard
https://xhr.spec.whatwg.org/
Other
314 stars 131 forks source link

Use XMLHttpRequestBodyInit as defined in Fetch, in send. #279

Closed gterzian closed 4 years ago

gterzian commented 4 years ago

Following-up on https://github.com/whatwg/fetch/pull/1029.

FIX #277

(See WHATWG Working Mode: Changes for more details.)


:boom: Error: 400 Bad Request :boom:

PR Preview failed to build. (Last tried on Jun 4, 2020, 6:22 AM UTC).

More PR Preview relies on a number of web services to run. There seems to be an issue with the following one: :rotating_light: [CSS Spec Preprocessor](https://api.csswg.org/bikeshed/) - CSS Spec Preprocessor is the web service used to build Bikeshed specs. :link: [Related URL](https://api.csswg.org/bikeshed/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fwhatwg%2Fxhr%2F379930757c7286965af541f2e013342ad24776b5%2Fxhr.bs&force=1&md-status=LS-PR&md-Text-Macro=PR-NUMBER%20279) _If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please [file an issue](https://github.com/tobie/pr-preview/issues/new?title=Error%20not%20surfaced%20properly&body=See%20whatwg/xhr%23279.)._
gterzian commented 4 years ago

This one fails to build locally, I assume the fetch PR needs to merge first for the idl link to work?

gterzian commented 4 years ago

cc @whatwg/documentation As MDN explicitly mentions ReadableStream(and BodyInit) as an option to XHR send, see https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/send

sideshowbarker commented 4 years ago

Updated https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/send

Lemme know if further changes/refinements are needed there

sideshowbarker commented 4 years ago

I guess the plan for WPT related to this includes removing or changing the https://github.com/web-platform-tests/wpt/blob/master/xhr/send-data-readablestream.any.js test?

gterzian commented 4 years ago

Updated https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/send

Thanks! By the way you could link to the XMLHttpRequestBodyInit in fetch as opposed to BodyInit, once https://github.com/whatwg/fetch/pull/1029/files merges...

I guess the plan for WPT related to this includes removing or changing the https://github.com/web-platform-tests/wpt/blob/master/xhr/send-data-readablestream.any.js test?

Yes, I think I will do it as part of https://github.com/servo/servo/issues/26723, where the changes to the test will be upstreamed.

annevk commented 4 years ago

We can merge this tomorrow, once Fetch has been indexed again.

annevk commented 4 years ago

@tabatkins Fetch exports XMLHttpRequestBodyInit as a typedef, that was done yesterday. Today XMLHttpRequest says it cannot find XMLHttpRequestBodyInit that is an idl-name. Was indexing delayed or is there a different problem?

tabatkins commented 4 years ago

This might have been caused by a Bikeshed error; I just fixed something and will see what happens as Shepherd cycles again.

tabatkins commented 4 years ago

And I'm seeing it in the db now, so you should be good to go.

annevk commented 4 years ago

@gterzian do we need implementation bugs here or are implementations already doing this? Firefox seems to be doing this, though a bug might still be good to get the IDL aligned.

gterzian commented 4 years ago

Ok, I have filed bugs(links added to the opening comment).

annevk commented 4 years ago

Amazing, thank you!

gterzian commented 4 years ago

@sideshowbarker pinging you again because since XMLHttpRequestBodyInit has now been added to Fetch, you could further update the docs by pointing to that one, instead of the previous BodyInit(the docs have ReadableStream removed, however still point to BodyInit).

sideshowbarker commented 4 years ago

since XMLHttpRequestBodyInit has now been added to Fetch, you could further update the docs by pointing to that one, instead of the previous BodyInit

OK, thanks — made that change