Fully merge iterable types into existing definitions#2431
Fully merge iterable types into existing definitions#2431jakebailey wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
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. |
There was a problem hiding this comment.
Pull request overview
This PR refactors how iterable types are emitted in the TypeScript DOM library generator. Previously, iterable types were appended as separate interface augmentations at the end of the generated files. Now, they are integrated directly into the main interface definitions.
Changes:
- Refactored iterator emission logic to emit iterable methods inline within interface declarations
- Added helper functions to determine and convert sequence/iterable types
- Modified parameter type conversion to use
Iterable<T>instead ofT[]when appropriate - Updated extends clauses to include iterator base types (e.g.,
Set<FontFace>,ReadonlyMap<K, V>)
Reviewed changes
Copilot reviewed 1 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/build/emitter.ts | Core refactoring: Added helper functions for iterable type detection and conversion; modified interface emission to inline iterable members; updated parameter type conversion logic |
| baselines/webworker.generated.d.ts | Generated baseline showing iterable methods merged into interface definitions instead of appended sections |
| baselines/sharedworker.generated.d.ts | Generated baseline with same pattern of merged iterable definitions |
| baselines/serviceworker.generated.d.ts | Generated baseline with same pattern of merged iterable definitions |
| baselines/audioworklet.generated.d.ts | Generated baseline with same pattern of merged iterable definitions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Not as nasty as I claimed, I think; it's just sort of the same code shifted outward. I went through and everything seems to be there for all declarations that moved / were replaced. |
|
One question, if we are going to merge them, why do we still need the *iterable.ts? |
|
The stubs exist because people could have written the name explicitly in tsconfig or reference directive. Otherwise <6.0 doesn't have this merge. (This PR just moves the declarations up from the bottom, since my original change for this was just a hack to tack them on at the bottom.) |
|
Got it, thanks for the reply |
|
microsoft/TypeScript#63193 implies this will cause more breaks. Nevermind. |
The code to do it seems nasty (largely just had copilot to it), but this does merge everything together instead of just tacking the types onto the end.