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.

Sign up for GitHub

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

Jump to bottom

Define a XMLHttpRequestBodyInit without ReadableStream #1029

Merged
merged 3 commits into from
Jun 2, 2020
Merged

Define a XMLHttpRequestBodyInit without ReadableStream #1029

merged 3 commits into from
Jun 2, 2020

Conversation

gterzian
Copy link
Member

@gterzian gterzian commented May 31, 2020 •
edited by pr-preview bot
Loading

Part of whatwg/xhr#277, removing ReadableStream from the body init used by XHR, and re-defining a BodyInit to include the stream option.

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

💥 Error: 400 Bad Request 💥

PR Preview failed to build. (Last tried on Jun 2, 2020, 4:27 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

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.

@gterzian gterzian changed the title define a XMLHttpRequestBodyInit without ReadableStream, re-define Bod… Define a XMLHttpRequestBodyInit without ReadableStream May 31, 2020
@gterzian gterzian mentioned this pull request May 31, 2020
Merged
3 tasks
@gterzian
Copy link
Member Author

gterzian commented May 31, 2020 •
edited
Loading

This one seems to build locally, is the error above pointing to an actual problem with the change? CI seems ok...

@yutakahirano
Copy link
Member

yutakahirano commented Jun 1, 2020 •
edited
Loading

Can you file a bug for each browser? You can refer to the same bugs here and in whatwg/xhr#279.

@yutakahirano
Copy link
Member

Ah sorry, no bugs are needed because this is specifying the current behavior. Can you state that in the description?

@gterzian
Copy link
Member Author

gterzian commented Jun 1, 2020 •
edited
Loading

Ok, I have added

None are filed because the changes here described the current behavior.

Reading MAINTAINERS.md, I think this might impact docs, MDN explicitly states a ReadableStream is an option, so I'll cc @whatwg/documentation and add the label.

Actually can someone add the "impacts documentation label "? Also it appears @whatwg/documentation is not an actual team?

@CYBAI
Copy link
Contributor

CYBAI commented Jun 1, 2020

Also it appears @whatwg/documentation is not an actual team?

IIRC, GitHub will only make the text mention-able for a team member's mention so maybe you will need someone (a team member of @whatwg/*)'s help to do the mention 👀

@gterzian
Copy link
Member Author

gterzian commented Jun 1, 2020

I have added the labels and cc'ed the team on the companion PR in XHR, as I think it's that one affecting docs rather than this one...

@annevk thank you for adding me to the org!

annevk
annevk approved these changes Jun 2, 2020
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me modulo this nit.

fetch.bs Outdated
typedef (Blob or BufferSource or FormData or URLSearchParams or ReadableStream or USVString) BodyInit;</pre>
typedef (Blob or BufferSource or FormData or URLSearchParams or USVString) XMLHttpRequestBodyInit;</pre>

<pre class=idl>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<pre class=idl>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, addressed...

@yutakahirano
Copy link
Member

cc: @yoichio fyi

@gterzian gterzian force-pushed the add_xhr_body_init branch from 7a0fb39 to 70be406 Compare June 2, 2020 15:44
annevk
annevk reviewed Jun 2, 2020
View reviewed changes
fetch.bs Outdated Show resolved Hide resolved
annevk
annevk reviewed Jun 2, 2020
View reviewed changes
fetch.bs Outdated Show resolved Hide resolved
@annevk annevk merged commit 9ac2b5b into whatwg:master Jun 2, 2020
@gterzian gterzian deleted the add_xhr_body_init branch June 2, 2020 16:32
annevk pushed a commit to whatwg/xhr that referenced this pull request Jun 4, 2020
@gterzian
Use XMLHttpRequestBodyInit as defined in Fetch, in send()
d9b16ed 
Following-up on whatwg/fetch#1029.

Tests: to be rewritten as part of servo/servo#26723.

Fixes #277.
@annevk annevk mentioned this pull request Jun 4, 2020
Closed
Bishwarupjee pushed a commit to Bishwarupjee/xhr that referenced this pull request Jan 31, 2024
@gterzian
Use XMLHttpRequestBodyInit as defined in Fetch, in send()
c9dbc79 
Following-up on whatwg/fetch#1029.

Tests: to be rewritten as part of servo/servo#26723.

Fixes #277.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk approved these changes

@yutakahirano yutakahirano yutakahirano approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gterzian @yutakahirano @CYBAI @annevk