Skip to content

Conversation

@Renegade334
Copy link
Contributor

@Renegade334 Renegade334 commented Jun 12, 2024

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
Copy link
Contributor

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
Copy link
Contributor

saschanaz commented Jun 12, 2024

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

@Renegade334
Copy link
Contributor Author

Renegade334 commented Jun 12, 2024

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
Copy link
Contributor

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

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

@Renegade334
Copy link
Contributor Author

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

@saschanaz
Copy link
Contributor

I guess interface is fine compared to that 😂

@saschanaz
Copy link
Contributor

(But please add some comment in addedTypes)

@HolgerJeromin
Copy link
Contributor

ref microsoft/TypeScript#23327 where the "interface" was added to be mergeable.

Copy link
Contributor

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@saschanaz
Copy link
Contributor

Thanks! LGTM

@github-actions github-actions bot merged commit c172a19 into microsoft:main Jun 14, 2024
@github-actions
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

3 participants