Update Streams to official WebIDL #890
Conversation
| "streampipeoptions-preventclose": "Pipes this readable stream to a given writable stream destination. The way in which the piping process behaves under various error conditions can be customized with a number of passed options. It returns a promise that fulfills when the piping process completes successfully, or rejects if any errors were encountered.\n\nPiping a stream will lock it for the duration of the pipe, preventing any other consumer from acquiring a reader.\n\nErrors and closures of the source and destination streams propagate as follows:\n\nAn error in this source readable stream will abort destination, unless preventAbort is truthy. The returned promise will be rejected with the source's error, or with any error that occurs during aborting the destination.\n\nAn error in destination will cancel this source readable stream, unless preventCancel is truthy. The returned promise will be rejected with the destination's error, or with any error that occurs during canceling the source.\n\nWhen this source readable stream closes, destination will be closed, unless preventClose is truthy. The returned promise will be fulfilled once this process completes, unless an error is encountered while closing the destination, in which case it will be rejected with that error.\n\nIf destination starts out closed or closing, this source readable stream will be canceled, unless preventCancel is true. The returned promise will be rejected with an error indicating piping to a closed stream failed, or with any error that occurs during canceling the source.\n\nThe signal option can be set to an AbortSignal to allow aborting an ongoing pipe operation via the corresponding AbortController. In this case, this source readable stream will be canceled, and destination aborted, unless the respective options preventCancel or preventAbort are set.", | ||
| "readablestreamiteratoroptions-preventcancel": "Asynchronously iterates over the chunks in the stream's internal queue.\n\nAsynchronously iterating over the stream will lock it, preventing any other consumer from acquiring a reader. The lock will be released if the async iterator's return() method is called, e.g. by breaking out of the loop.\n\nBy default, calling the async iterator's return() method will also cancel the stream. To prevent this, use the stream's values() method, passing true for the preventCancel option.", | ||
| "queuingstrategyinit-highwatermark": "Creates a new ByteLengthQueuingStrategy with the provided high water mark.\n\nNote that the provided high water mark will not be validated ahead of time. Instead, if it is negative, NaN, or not a number, the resulting ByteLengthQueuingStrategy will cause the corresponding stream constructor to throw." | ||
| } |
There was a problem hiding this comment.
This comment map is very broken. The Streams standard does have comments, but it doesn't look like idlfetcher understands the comment style.
Do we want to fix idlfetcher to handle the comment style of the Streams standard? Or do we want to (somehow) disable comment maps for Streams, and leave them undocumented for now?
There was a problem hiding this comment.
The comment generator currently has some issues on other specs too, we probably want to fix it.
There was a problem hiding this comment.
I'm trying a couple of things to improve it.
Expect a separate PR soon. See PR #894. 😁
|
This is (finally) ready for review. 😄
Apologies for the large number of force-pushes! I try to keep the commits linear and easy to review, which means a lot of rebases to stay up-to-date with master. 😅 |
There was a problem hiding this comment.
After discussing the intent of the PR, I think we should take this. Streams doesn't seem well supported, but we already include it, so it should be kept up to date. (And @orta suspects that it's used by some popular tools even if it's V8-only.)
The changes themselves seem ok? They look reasonable for how little knowledge I have of the standard.
|
@MattiasBuelens now that Chrome ships with Byte Streams enabled, can we resurrect them in the TS typings? |
|
@trxcllnt Good point! We initially removed the types for readable byte streams since they weren't implemented by any browser yet, but now they have shipped in Chromium-based browsers. I'll look into it. 🙂 |
|
Actually no, it needs to be on at least two engines to be enabled. |
Right, I remember now! Guess we'll have to wait for Firefox and Safari to show some love for readable byte streams. 😅 |
|
caniuse reports byte streams are enabled for 65% of users. Yes they're all based on Blink, but (if accurate) that's still a huge number. The main issue is the types existed at one point and now don't. We wrote libs anticipating native byte streams would eventually ship in some form, told folks to use @MattiasBuelens's I understand updating as changes are made in the whatwg spec, but they haven't been removed there. Perhaps they never should've been added here, but that ship has sailed. |
|
I understand your frustration. With types for readable byte streams removed from TypeScript 4.2 and In retrospect, we should have spotted that this feature wasn't sufficiently supported by browsers, and kept those types out when we added streams to TypeScript's DOM types. But as you said, that ship has sailed. 😞 Is it possible to resurrect |
It looks like this is the idea of #1023. I'm already a fan! 😁 |
|
@MattiasBuelens what do you recommend us to do now if we would like to use byte streams? Can you resurrect |
Update the types for the Streams standard to use the official WebIDL definitions.
I'm already opening this PR to have a better idea of what sort of (breaking) changes this would cause, and to continue the discussion in #886. Here's what I did so far:
PipeOptions➡️StreamPipeOptionsandReadableStreamErrorCallback➡️UnderlyingSourceCancelCallback. This is technically a breaking change, but I don't expect many users will be using these dictionary or callback types directly.ReadableStream. This was not yet part of the generated types, but it is part of the new IDL. Currently, we would incorrectly recognize this as a regular iterable declaration, which would result in incorrect generated code (e.g. aforEachmethod that doesn't exist in practice). See Update Streams to spec version 46c3b89 #669 and Support async iterable DOM types TypeScript#29867.WritableStream.close. This method did not yet exist in the type definitions, but I explicitly prevent it from being added. We'll wait until more browsers implement it before adding it (by "removing the removal"). See Add WritableStream.close() #827.ReadableStreamReadResulttoReadableStreamDefaultReadResult. The standard also definesReadableStreamBYOBReadResultbut, again, I removed byte stream support.done: truecase forReadableStreamDefaultReadResultto always hasvalue: undefined. The only case when you could have a value is when using a BYOB reader, which we do not want to support yet.ReadableStreamReaderto be a type alias forReadableStreamDefaultReader. In the future, this will become a type union to includeReadableStreamBYOBReader, as per spec.I'd like some input regarding what we should and should not change.
WritableStream,TransformStreamandpipeTo/pipeThrough, even though only Chromium has implemented them? These types are already used in several projects, and it would be a bit of a shame if they'd need to go back to using third-party type definitions for these.WritableStreamandTransformStream, this feature is actually not supported anywhere, so I don't expect many users to be using these types in practice.Depends on #920.
Fixes #886.