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

Closed
wants to merge 2 commits into 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 [PR only] 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

github-actions bot commented May 25, 2024

Preview URLs

(comment last updated: 2024-07-02 05:04:30)

@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:

  1. add a link to https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Concepts from https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultController/enqueue#see_also
  2. add a link to https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Concepts from the sidebar for this page? I filed Show guide pages in {{APIRef}} macro yari#6229 for this a long time ago but Mozilla haven't prioritised it.
  3. link directly from "chunk" here to https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Concepts ? It's a bit odd because we can't link directly to a definition of "chunk" in that page, just because of how that page is structured.
  4. we could additionally restructure https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Concepts so it provides a linkable definition of "chunk", and that might be the best option, but it's quite a bit more work.

@Josh-Cena
Copy link
Member

@Mubelotix Are you going to apply one of the suggestions above?

@Josh-Cena Josh-Cena added the awaiting response Awaiting for author to address review/feedback label Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Awaiting for author to address review/feedback Content:WebAPI Web API docs size/xs [PR only] 0-5 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants