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

Add Streams types #521

Closed
saschanaz opened this issue Jun 29, 2018 · 7 comments
Closed

Add Streams types #521

saschanaz opened this issue Jun 29, 2018 · 7 comments

Comments

@saschanaz
Copy link
Contributor

saschanaz commented Jun 29, 2018

The Streams spec does not provide any Web IDL so it has to be added through the manual way.

cc: @surma

@saschanaz
Copy link
Contributor Author

Alternatively we can craft our own IDL, which should be more readable.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 29, 2018

Should we file a spec bug for that?

@saschanaz
Copy link
Contributor Author

There already is: whatwg/streams#45

The author isn't happy with Web IDL...

@mhegazy
Copy link
Contributor

mhegazy commented Jun 29, 2018

Alternatively we can craft our own IDL

Well.. according to microsoft/TypeScript#25277 we need to make it generic. do not think WebIDL can do that today, so we will have to have a base webIdl with a punch of any's and then a modification layer on top. might be easier to just have a .d.ts file that we include verbatim.

@surma
Copy link

surma commented Jun 29, 2018

Where do the current definitions for ReadableStream and WritableStream come from?

@MattiasBuelens
Copy link
Contributor

I want to give this a try! 😄 I've already started with a few types to see what could work, and I need some suggestions on how to proceed.


Ideally, we'd take the existing .d.ts from DefinitelyTyped and merge it into dom.generated.d.ts. However, it seems that the lib generator doesn't work like that: it only consumes WebIDL files or modifications from {added,overriding,removed}Types.json files. As @mhegazy already mentioned, it seems like we should either:

  • Rewrite the .d.ts as WebIDL, and add some tweaks in the JSON files for types that can't be expressed in WebIDL. This will not be an official WebIDL file, which might be a bit of a risk, although we'll have to tweak and maintain the types anyway.
  • Rewrite the .d.ts entirely as JSON. This will probably be quite verbose, but you wouldn't need to go back-and-forth between WebIDL and JSON to figure out where a type is defined.

Any suggestions on what approach is more desirable would be much appreciated.


The read method of ReadableStreamDefaultReader currently has the following type:

read(): Promise<IteratorResult<R>>;

However, IteratorResult is defined in es2015.iterable, which is not available when targeting ES5 or lower.

I think we can get away with defining a separate interface (e.g. ReadResult<R>) that is just a copy of IteratorResult<R>. Since TypeScript uses structured typing, the type checker should treat both as equivalent. Or is there a cleaner way to handle this?


I noticed that many WritableStream types are already defined (because they're implemented in Edge), but some of them are quite awkward. For example, the callbacks in UnderlyingSink are defined as callable interfaces. I think this is because they're generated from a WebIDL callback definition.

Is it okay if we change these to be function types? For example:

// old
interface WritableStreamDefaultControllerCallback {
    (controller: WritableStreamDefaultController): void;
}
// new
type WritableStreamDefaultControllerCallback = (controller: WritableStreamDefaultController) => void;

Or should we keep using callable interfaces, including for the new callbacks (e.g. UnderlyingSource.pull)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants