search

microsoft / TypeScript-DOM-lib-generator

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

Add override for `import.meta.resolve()` #1740

Closed Renegade334 closed 3 weeks ago

Renegade334 commented 3 weeks ago

Specification: HTML Standard 8.1.6.5.1 Reference: MDN Reference: microsoft/TypeScript#58828

Converting ImportMeta to an interface seems fairly unavoidable, but I don't think it should break anything.

github-actions[bot] commented 3 weeks 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 3 weeks ago

Maybe the extra field can have a lambda function type to make it simpler while keeping dictionary?

Renegade334 commented 3 weeks ago

Maybe the extra field can have a lambda function type to make it simpler while keeping dictionary?

Considered this, but this approach leads to interface merging conflicts in places like @types/node. As a result, it really needs to be defined as a method rather than a property.

saschanaz commented 3 weeks ago

Oh no, but ImportMeta is not an interface at all. 😞

Can you at least add some comment why it should be an interface?

Renegade334 commented 3 weeks ago

The only alternative I can see is the deliciously hacky e341817...

saschanaz commented 3 weeks ago

I guess interface is fine compared to that 😂

saschanaz commented 3 weeks ago

(But please add some comment in addedTypes)

HolgerJeromin commented 3 weeks ago

ref https://github.com/microsoft/TypeScript/pull/23327 where the "interface" was added to be mergeable.

saschanaz commented 3 weeks ago

Thanks! LGTM

github-actions[bot] commented 3 weeks ago

Merging because @saschanaz is a code-owner of all the changes - thanks!