Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions inputfiles/overridingTypes.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
}
}
},
"ChildNode": {
"extends": "Node"
},
"ParentNode": {
"extends": "Node"
},
Expand Down
1 change: 1 addition & 0 deletions inputfiles/patches/childnode.kdl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
interface-mixin ChildNode extends="Node"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(skip quote)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I don't think it's worth having a separate file for a single line change, but then I'm not sure what would be the right name. Maybe misc.kdl is fine and we can put random simple changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think extensions is a good name, because there are a bunch of things that we extend

Copy link
Contributor

@saschanaz saschanaz Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And please cover ParentNode too, and some comment like:

// ChildNode and ParentNode are actually defined as mixins, but because of their names they have historically been used as a sub-interface of Node.

... with some line wrapping.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think extensions is a good name, because there are a bunch of things that we extend

Actually I change my mind here, covering ChildNode/ParentNode with some context is worth a separate file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@saschanaz saschanaz Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now child-parent-node.kdl or such? With that LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done @saschanaz

8 changes: 6 additions & 2 deletions src/build/patches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,15 @@ function handleMixin(node: Node): DeepPartial<Interface> {
}
}

return {
const result: any = {
name,
events: { event },
properties: { property },
} as DeepPartial<Interface>;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const result: any = {
name,
events: { event },
properties: { property },
} as DeepPartial<Interface>;
};
const result = {
name,
events: { event },
properties: { property },
} as DeepPartial<Interface>;

It's not clear to me why const result: DeepPartial<Interface> doesn't work (the error is confusingly complex), but this should be ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if (node.properties.extends) {
result.extends = node.properties.extends;
}
return result as DeepPartial<Interface>;
}

/**
Expand Down