Refactor mixin handling in KDL parser #2082
Conversation
- Removed AbstractWorker event definitions from addedTypes.jsonc. - Introduced abstractworker.kdl to define AbstractWorker mixin with error event. - Updated patches.ts to handle mixin parsing and return mixins in the output.
|
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. |
| @@ -0,0 +1,3 @@ | |||
| mixin AbstractWorker event { | |||
| error ErrorEvent | |||
There was a problem hiding this comment.
| error ErrorEvent | |
| event error type=ErrorEvent |
| @@ -0,0 +1,3 @@ | |||
| mixin AbstractWorker event { | |||
There was a problem hiding this comment.
| mixin AbstractWorker event { | |
| interface-mixin AbstractWorker { |
|
Thank you @saschanaz |
|
Briefly something like in #2053 (comment). Could change during review in case something is off though. // This part is done, thank you 🎉
enum Foo {
foo
bar
}
interface Foo {
// Event information should ideally be provided by MDN and @webref/events, this is for edge cases
event show type=ShowEvent
event close type=CloseEvent
method fooOp returns=FooReturnType {
param arg1 type=ArgType
}
property fooAttr type=AttrType
}
interface Foo2 {
template T1 extends=FooFoo default=FooBar
operation fooOp override-returns=TemplateType<T1>
}
dictionary Bar {
template T
member bar required=#true type=DOMString
// but all types here are to override the IDL one, perhaps there could be a better word, like ts-type?
member baz override-type=Foo2<T>
}
|
- Simplified event array creation using map instead of forEach. - Improved readability and performance of the event handling logic.
|
I have updated it @saschanaz. |
src/build/patches.ts
Outdated
| * @param node The mixin node to handle. | ||
| * @param mixins The record of mixins to update. | ||
| */ | ||
| export function handleMixin(node: any, mixins: Record<string, any>) { |
There was a problem hiding this comment.
| export function handleMixin(node: any, mixins: Record<string, any>) { | |
| function handleMixin(node: Node, mixins: Record<string, any>) { |
(Something I missed last time)
| @@ -0,0 +1,3 @@ | |||
| interface-mixin AbstractWorker { | |||
| event error type=ErrorEvent | |||
| } | |||
There was a problem hiding this comment.
Would be nice to have a final newline - if you use VSCode then "Files: Insert Final Newline" should do it for you.
| @@ -0,0 +1,3 @@ | |||
| interface-mixin AbstractWorker { | |||
There was a problem hiding this comment.
In this specific case, I'd prefer to put all the event things to a single file as events.kdl, so that I can see which events are not covered by automation and can ultimately remove this file.
In this PR just the filename can change, no addition is needed.
(Because all event data technically exists, we don't have a way to track that to assign automatically right now)
src/build/patches.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| import { parse } from "kdljs"; | |||
There was a problem hiding this comment.
| import { parse } from "kdljs"; | |
| import { parse, type Node } from "kdljs"; |
|
Thank you for your review @saschanaz |
|
LGTM, thanks! |
|
There was an issue merging, maybe try again saschanaz. Details |
|
LGTM |
|
Merging because @saschanaz is a code-owner of all the changes - thanks! |
Related #2053