search

microsoft / TypeScript-DOM-lib-generator

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

Add [Symbol.toStringTag] property to all interfaces #1699

Open jdufresne opened 6 months ago

jdufresne commented 6 months ago

Fixes #1641

github-actions[bot] commented 6 months 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.

jdufresne commented 6 months ago

I made an attempt but I'm hoping to receive a bit guidance and early feedback on the approach and how to proceed. Running tests results in errors that look like:

generated/dom.generated.d.ts(28925,18): error TS1169: A computed property name in an interface must refer to an expression whose type is a literal type or a 'unique symbol' type.
generated/dom.generated.d.ts(28925,19): error TS2585: 'Symbol' only refers to a type, but is being used as a value here. Do you need to change your target library? Try changing the 'lib' compiler option to es2015 or later.

I'm not sure how to reconcile it.

jdufresne commented 6 months ago

@microsoft-github-policy-service agree

jdufresne commented 6 months ago

Thanks for the feedback. I took your advice and tried to follow the existing iterable pattern.

sandersn commented 5 months ago

I don't much like the implementation since it requires so many new files. Also, I don't understand how this is useful to anybody. It's not a property that you need to refer to directly, is it?

Following the links in the original bug and skimming through them, it seems like there's some complex assignability problems without this property. Would it be possible to fix those in a simpler way (or stop using this property entirely -- I don't think it's meant for instanceof checks in the first place).

@saschanaz I skimmed the webidl spec discussion but couldn't find a definitive place that says that these are part of the DOM. How important do you think it is for spec compliance to have them here?

saschanaz commented 5 months ago

@saschanaz I skimmed the webidl spec discussion but couldn't find a definitive place that says that these are part of the DOM. How important do you think it is for spec compliance to have them here?

By DOM I guess you meant Web Platform? Anything in Web IDL spec is part of Web Platform in that case, otherwise it won't be there.

About the size, given that the new files are pretty much duplicated with same symbolic member definitions, perhaps there can be an empty interface WebIDLInterface {} that extends every interface definition in es5 lib and then es6 lib - well, dom.iterable.d.ts - can add a member on it`. Then we don't have to add new files.

saschanaz commented 5 months ago

About how important, I think it's pretty much edge case to use this at all, so maybe not too much worth extra effort 🤔

jdufresne commented 2 months ago

Just a heads up. I'm not actively working on this at the moment. If someone would like to pick up the remaining work to take it over the finish line, that is welcome.