Support async_sequence - #2533
Conversation
|
Please just do the async_sequence part, the other parts are too big to consume. |
Sure, but how can we test it without upgrading the library? |
|
That's a good question. But it's kinda more important to make the job pass than making it correct right now, we'll get it checked later when we get the actual IDL updated. |
Signed-off-by: Bashamega <adambashaahmednaji@gmail.com>
Done. Thanks for looking at this. When will you check my previous PRs? Or should I close them? |
| } | ||
| return "AsyncIterable"; | ||
| } else { | ||
| // Legacy main & fallback builds: no async iterable interface available. |
There was a problem hiding this comment.
What is main & fallback builds? Can we say legacy TS versions?
| if (compilerBehavior.treatAsyncSequence) { | ||
| // Forwards-compatible definition: | ||
| // NOTE: In IDL→TS (whatwg/streams#1372 etc) async_sequence<T> -> AsyncIterable<T> | ||
| // If stricter interop wanted, could use AsyncIterable<T> | Iterable<T> |
There was a problem hiding this comment.
Why?
| // For TS <2.3, the generator separates out [Symbol.asyncIterator] into *.asynciterable.d.ts, so | ||
| // these references must not leak into the main file, so we can treat them as `any` as a fallback. | ||
| if (compilerBehavior.treatAsyncSequence) { | ||
| // Forwards-compatible definition: |
There was a problem hiding this comment.
forwards-compatible?
| // Support async_sequence (see https://github.com/whatwg/streams/pull/1372) | ||
| if (obj.type === "async_sequence") { | ||
| // If we're in a build that includes async iterables (TS 2.3+ or forced), emit as AsyncIterable<T>. | ||
| // For TS <2.3, the generator separates out [Symbol.asyncIterator] into *.asynciterable.d.ts, so |
There was a problem hiding this comment.
I don't think we care about TS < 2.3, minimum supported version is already 4.4.
That said, AsyncIterable does require separate asynciterable.d.ts until TS 6.0, so to make it correct we need to defer any functions that uses async_sequence to asynciterable variant.
Given we don't really test it here, it's also acceptable to just entirely skip the generation for those functions with some console warning.
There was a problem hiding this comment.
I don't think we are doing this?
There was a problem hiding this comment.
I don't quite understand this comment but i have pushed a change tell me if this is what you mean
| outputFolder: new URL("./ts5.5/", outputFolder), | ||
| compilerBehavior: {}, // ts5.5 does not support `IteratorObject` or unrelated setter types | ||
| compilerBehavior: { | ||
| treatAsyncSequence: false, |
There was a problem hiding this comment.
Let it just empty as others did here
|
done |
… warn for TypeScript versions below 6.0
| compilerBehavior: { | ||
| useIteratorObject: true, | ||
| allowUnrelatedSetterType: true, | ||
| treatAsyncSequence: false, |
There was a problem hiding this comment.
Again, please skip it as others do
There was a problem hiding this comment.
Sorry, done
| const type = convertBaseType(); | ||
| if (type === "any") { | ||
| return type; | ||
| } |
There was a problem hiding this comment.
Does this still matter?
closes #2480