-
Notifications
You must be signed in to change notification settings - Fork 459
Migrate IntersectionObserverEntry to KDL #2314
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. |
Can you eleborate, like what kind of error? Working around an error is only good when we know what's happening there. |
Scratch that, I have updated the syntax |
| } | ||
| if (signatureObj.param?.length === 0 && !signatureObj.type) { | ||
| // If there are no params and no return type, remove the signature | ||
| signature = undefined; |
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.
This would cause problem for non-removal patches, no?
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.
This wouldn't cause an issue; we already have patches for non-removal methods, and it is working prefectly
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 mean if we want to add a signature... Hmm, but doing so would require a type, so maybe this is fine.
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, so this is good. Anything else?
Also, I added full support for interface removals.
Everything now should be optionally nested
except for the signature, because the emitter throws an error otherwise. So I had to add special handling for it in the converter