Skip to content

[whatwg-streams] Fixes for readable byte streams#27375

Merged
weswigham merged 6 commits intoDefinitelyTyped:masterfrom
MattiasBuelens:whatwg-streams-byte-fixes
Jul 21, 2018
Merged

[whatwg-streams] Fixes for readable byte streams#27375
weswigham merged 6 commits intoDefinitelyTyped:masterfrom
MattiasBuelens:whatwg-streams-byte-fixes

Conversation

@MattiasBuelens
Copy link
Copy Markdown
Contributor

@MattiasBuelens MattiasBuelens commented Jul 17, 2018

This PR makes the type definitions for readable byte streams match the spec more closely. An overview of the changes:

  • ReadableByteStreamController.enqueue accepts any ArrayBufferView.
    Step 5 only requires chunk to have a [[ViewedArrayBuffer]] internal slot, which exactly matches ArrayBuffer.isView. Different chunks can have different types, as long as each of them is some kind of ArrayBufferView.
  • ReadableStreamBYOBRequest.view is always a Uint8Array.
    Step 2b always constructs a Uint8Array. It will never be a different kind of ArrayBufferView.
  • ReadableByteStreamController.byobRequest may be undefined.
    Step 2 only constructs a BYOB request when [[pendingPullIntos]] is not empty. If it is empty, no BYOB request is constructed.
  • Remove unused type parameter on ReadableByteStreamSource and related classes.
    As demonstrated by the above changes, readable byte streams aren't generic according to the spec, so there's no use for the R type parameter.

I've also included a small ergonomic fix for users targeting ES5:

  • Replace IteratorResult with own interface to avoid dependency on ES2015 types.
    Previously, we used IteratorResult<T> in the return type of read(), but this type is not available when compiling with --lib es5. I've added a ReadResult<T> type which is just a copy of IteratorResult<T>, so the type definitions also work in ES5. 😄

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: [https://streams.spec.whatwg.org]
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

@typescript-bot typescript-bot added Awaiting reviewer feedback Author is Owner The author of this PR is a listed owner of the package. labels Jul 17, 2018
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Jul 17, 2018

@MattiasBuelens Thank you for submitting this PR!

🔔 @saschanaz @ksm2 - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot
Copy link
Copy Markdown
Contributor

Since you're a listed owner and the build passed, this PR is fast-tracked. A maintainer will merge shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@weswigham weswigham merged commit 1d3d5b2 into DefinitelyTyped:master Jul 21, 2018
@MattiasBuelens MattiasBuelens deleted the whatwg-streams-byte-fixes branch July 21, 2018 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author is Owner The author of this PR is a listed owner of the package.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants