- 
                Notifications
    You must be signed in to change notification settings 
- Fork 453
          Emit Symbol.toStringTag properties in TS 6.0
          #2193
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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. | 
a4864a2    to
    62a6dcf      
    Compare
  
            
          
                inputfiles/overridingTypes.jsonc
              
                Outdated
          
        
      | }, | ||
| "DOMException": { | ||
| "extends": "Error" | ||
| "extends": "Error", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move DOMException to a .kdl file, maybe move it to dom.kdl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving the existing override is really beyond the scope of this changeset; would be better suited to a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving the existing override is really beyond the scope of this changeset; would be better suited to a follow-up PR.
Then remove the changes in patches.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why noToStringTag at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error doesn't have a toStringTag property in the TS libs, and DOMException is a common target for userland inheritance, so the concern is that this will do more harm than good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess Jake needs to make such PR then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, getting there 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The results on the above are pretty bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A significant proportion are intentional errors that are now differentiating between discrete DOM interfaces that are empty or identically-shaped, but looks like most of the remainder are assignability errors between the lib.dom interfaces and userland DOM-like interfaces that are unlikely to go down well. (It's worth saying that those same errors would also be triggered by those interfaces having any new props/methods added, not just toStringTag, but adding an additional key to every single interface would turbocharge the breakage.)
2ea828a    to
    28dd4e8      
    Compare
  
    28dd4e8    to
    a523b16      
    Compare
  
    
Fixes #1641.
Fixes #2172.
Alternative to #1762.
Supersedes #2075.
Reference: https://webidl.spec.whatwg.org/#dfn-class-string
This amends the builder to append
Symbol.toStringTagproperties to interfaces and iterators in the ts6.0 core libraries. (ES6's well-known symbols are universally defined in ts6.0.)Because adding a property with a literal type affects inheritance chains, only "final" classes (ie. those that do not have any inheritance children within the current global) can have these properties typed as string literals. (For example, it's not possible for
Blob#[Symbol.toStringTag]to be typed as"Blob", because File inherits from Blob, and a property of type"File"can't override a parent property of type"Blob".)For those interfaces that have one or more inheritance children, there are two possible approaches.
Symbol.toStringTagproperties, and others (including some very common ones) without them.string, so that they can be correctly overridden by child interfaces. This is the approach taken here.The toStringTag emitter does not add to any interface marked
noInterfaceObject, since these are usually pseudo-types that are not true IDL interfaces, nor to namespaces converted to interfaces (ie.console).