-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
fix(types): Conform lib.deno_web.d.ts to lib.dom.d.ts and lib.webworker.d.ts #24599
Conversation
Will this also fix? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. Could you please add some tests like I did in #24489 to ensure we don't regress again?
537d04b
to
1695f7a
Compare
@petamoriken do you need any help with this PR? I'd like to land it before v1.45.3 release that will happen on Monday, July 22nd. |
@bartlomieju Finally, I am going to add type aliases for users who refer to old types. It shouldn't take that long. |
/** @category Streams */ | ||
declare interface ReadableStreamBYOBReadDoneResult<V extends ArrayBufferView> { | ||
done: true; | ||
value?: V; | ||
} | ||
|
||
/** @category Streams */ | ||
declare interface ReadableStreamBYOBReadValueResult<V extends ArrayBufferView> { | ||
done: false; | ||
value: V; | ||
} | ||
|
||
/** @category Streams */ | ||
declare type ReadableStreamBYOBReadResult<V extends ArrayBufferView> = | ||
| ReadableStreamBYOBReadDoneResult<V> | ||
| ReadableStreamBYOBReadValueResult<V>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be reland when supported by upstream (microsoft/TypeScript-DOM-lib-generator#1676)
readonly readable: ReadableStream<string>; | ||
readonly writable: WritableStream<BufferSource>; | ||
readonly [Symbol.toStringTag]: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks a lot!
Fix #24578
Fix #21981