search

microsoft / TypeScript-DOM-lib-generator

Tool for generating dom related TypeScript and JavaScript library files
Apache License 2.0
616 stars 417 forks source link

Updating added types for `ReadableStreamReadDoneResult` #1676

Open kraenhansen opened 8 months ago

kraenhansen commented 8 months ago

This fixes #1675.

github-actions[bot] commented 8 months ago

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

saschanaz commented 8 months ago

Makes sense, bu you need to build and commit the result.

saschanaz commented 8 months ago

Actually byte streams indeed return the value, btw. Perhaps split the type? https://streams.spec.whatwg.org/#byob-reader-internal-slots

saschanaz commented 8 months ago

Hi @MattiasBuelens, would you be satisfied with this?

Also it would be good to have some unit tests here.

kraenhansen commented 8 months ago

I've updated the code to separate the ReadableStreamReadResult into two:

I've also added ReadableStreamReadResult which union both, to keep breaking changes to a minimum.

The two new types reuse ReadableStreamReadValueResult but have their separate "DoneResult" types:

The PR is currently failing tests:

Test failed: could not compile 'dom.generated.d.ts':
generated/dom.generated.d.ts(28201,75): error TS2552: Cannot find name 'ReadableStreamBYOBReadDoneResult'. Did you mean 'ReadableStreamBYOBReadResult'?
generated/dom.generated.d.ts(28203,78): error TS2552: Cannot find name 'ReadableStreamDefaultReadDoneResult'. Did you mean 'ReadableStreamDefaultReadResult'?

I can't seem to get the specialised *DoneResult types into the dom.generated.d.ts 🤔

MattiasBuelens commented 8 months ago

The PR is currently failing tests:

Test failed: could not compile 'dom.generated.d.ts':
generated/dom.generated.d.ts(28201,75): error TS2552: Cannot find name 'ReadableStreamBYOBReadDoneResult'. Did you mean 'ReadableStreamBYOBReadResult'?
generated/dom.generated.d.ts(28203,78): error TS2552: Cannot find name 'ReadableStreamDefaultReadDoneResult'. Did you mean 'ReadableStreamDefaultReadResult'?

I can't seem to get the specialised *DoneResult types into the dom.generated.d.ts 🤔

The Streams types have [Exposed=*] in their IDL, so I think you need to add "exposed": "*" (or something equivalent) to your addedTypes.jsonc changes?

kraenhansen commented 8 months ago

I can't seem to get the specialised *DoneResult types into the dom.generated.d.ts 🤔

I found the issue 👍