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

Clarify the type of chunk parameter in ReadableStreamDefaultController #33562

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Mubelotix
Copy link

Source: The spec https://streams.spec.whatwg.org/#model

Description

Clarify the type of chunk parameter in ReadableStreamDefaultController

Motivation

I had no idea what object I was supposed to pass

@Mubelotix Mubelotix requested a review from a team as a code owner May 12, 2024 16:55
@Mubelotix Mubelotix requested review from wbamberg and removed request for a team May 12, 2024 16:55
@github-actions github-actions bot added Content:WebAPI Web API docs size/xs 0-5 LoC changed labels May 12, 2024
@Mubelotix
Copy link
Author

What's going on here? Why isn't anyone reviewing this?

Copy link
Contributor

@wbamberg
Copy link
Collaborator

What's going on here? Why isn't anyone reviewing this?

Sorry, we have a lot to do here and it takes a while to get to reviews sometimes. I will take a look tomorrow.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, @Mubelotix , but I don't think this is the right thing to do. "chunk" is a really fundamental concept in the Streams API, and it's assumed that people reading the reference docs for the various parts of it have that background knowledge.

Put another way: there are lots and lots of references to "chunk" throughout the Streams API reference documentation, and I don't think it makes sense to define it inline every time it is used.

We do define "chunk" in the introductory guide material for the API (https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Concepts).

We could look at making the linkage to that more obvious? Options might include:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/xs 0-5 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants