-
Notifications
You must be signed in to change notification settings - Fork 469
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?
Changes from 6 commits
9a179b6
61c09a1
2846591
fe51565
706609a
5eddd2d
c91c7a4
ca7bf6b
4c7befe
ae01428
6a7373b
db959a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,8 +245,8 @@ function handleMixinAndInterfaces( | |
| return { | ||
| name, | ||
| ...optionalNestedMember("events", event, { event }), | ||
| properties: { property }, | ||
| methods: { method }, | ||
| ...optionalNestedMember("properties", property, { property }), | ||
| ...optionalNestedMember("methods", method, { method }), | ||
| ...optionalMember("extends", "string", node.properties?.extends), | ||
| ...optionalMember("overrideThis", "string", node.properties?.overrideThis), | ||
| ...optionalMember("forward", "string", node.properties?.forward), | ||
|
|
@@ -353,7 +353,7 @@ function handleMethodAndConstructor( | |
| const signatureIndex = child.properties?.signatureIndex; | ||
| const type = handleTyped(typeNodes, child.properties?.returns); | ||
|
|
||
| let signature: OverridableMethod["signature"] = []; | ||
| let signature: OverridableMethod["signature"] | undefined = undefined; | ||
| if (type || params.length > 0) { | ||
| // Determine the actual signature object | ||
| const signatureObj: DeepPartial<Signature> = { | ||
|
|
@@ -365,6 +365,10 @@ function handleMethodAndConstructor( | |
| } else { | ||
| signature = [signatureObj]; | ||
| } | ||
| if (signatureObj.param?.length === 0 && !signatureObj.type) { | ||
| // If there are no params and no return type, remove the signature | ||
| signature = undefined; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would cause problem for non-removal patches, no?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, so this is good. Anything else?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not "good" but more like "I can live with this for now" for me... but yeah this should be okay.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but constructors don't have a return type, so (Meaning we should throw if type exists for constructor btw)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will open another one of the throwing an error. It wouldn't count as a removal because the parameters array will be filled. We check for both
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe remove it when signatureIndex exists? Having signatureIndex without having params or types don't make sense. (In that case we'll only remove the constructor/method's signature but not themselves, which I think is fine as it should ultimately behave same) |
||
| } | ||
| } | ||
| return { | ||
| name, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.