-
Notifications
You must be signed in to change notification settings - Fork 453
Feature: Add method handling to mixin processing in KDL #2100
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
Changes from 20 commits
a62a343
e3f0a20
d7e96fb
a88fbed
2fcb5b9
e6da694
37825cf
c095db7
9682083
6aa05dd
8f7ca28
0741374
7338aab
333fb26
ea16eb1
fcd4188
c5dcd9e
58e6d23
8c2aa44
03d4afe
2cfa765
bd68c2d
0693d5d
12566ec
7da25a0
c4ba8c8
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 |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// Manually moved from Document | ||
// See https://github.com/w3c/csswg-drafts/issues/5886 and https://github.com/w3c/csswg-drafts/issues/556 | ||
interface-mixin DocumentOrShadowRoot { | ||
method elementFromPoint { | ||
type Element nullable=#true | ||
param x type=long | ||
param y type=long | ||
} | ||
method elementsFromPoint { | ||
type sequence { | ||
type Element | ||
} | ||
param x type=long | ||
param y type=long | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,13 @@ | ||
import { parse, type Value, type Node } from "kdljs"; | ||
import type { Enum, Event, Property, Interface, WebIdl } from "./types.js"; | ||
import type { | ||
Enum, | ||
Event, | ||
Property, | ||
Interface, | ||
WebIdl, | ||
Method, | ||
Typed, | ||
} from "./types.js"; | ||
import { readdir, readFile } from "fs/promises"; | ||
import { merge } from "./helpers.js"; | ||
|
||
|
@@ -25,6 +33,25 @@ function optionalMember<const T>(prop: string, type: T, value?: Value) { | |
}; | ||
} | ||
|
||
function string(arg: unknown): string { | ||
if (typeof arg !== "string") { | ||
throw new Error(`Expected a string but found ${typeof arg}`); | ||
} | ||
return arg; | ||
} | ||
|
||
function handleTyped(type: Node, returnType?: Value): Typed { | ||
const isTyped = type.name == "type"; | ||
const name = string(isTyped ? type.values[0] : returnType); | ||
const subType = | ||
type.children.length > 0 ? handleTyped(type.children[0], name) : undefined; | ||
return { | ||
type: name, | ||
subtype: subType, | ||
...optionalMember("nullable", "boolean", type.properties?.nullable), | ||
}; | ||
} | ||
|
||
/** | ||
* Converts patch files in KDL to match the [types](types.d.ts). | ||
*/ | ||
|
@@ -40,10 +67,7 @@ function parseKDL(kdlText: string): DeepPartial<WebIdl> { | |
const mixin: Record<string, DeepPartial<Interface>> = {}; | ||
|
||
for (const node of nodes) { | ||
const name = node.values[0]; | ||
if (typeof name !== "string") { | ||
throw new Error(`Missing name for ${node.name}`); | ||
} | ||
const name = string(node.values[0]); | ||
switch (node.name) { | ||
case "enum": | ||
enums[name] = handleEnum(node); | ||
|
@@ -66,10 +90,7 @@ function parseKDL(kdlText: string): DeepPartial<WebIdl> { | |
* @param enums The record of enums to update. | ||
*/ | ||
function handleEnum(node: Node): Enum { | ||
const name = node.properties?.name || node.values[0]; | ||
if (typeof name !== "string") { | ||
throw new Error("Missing enum name"); | ||
} | ||
const name = string(node.properties?.name || node.values[0]); | ||
const values: string[] = []; | ||
|
||
for (const child of node.children) { | ||
|
@@ -88,23 +109,26 @@ function handleEnum(node: Node): Enum { | |
*/ | ||
function handleMixin(node: Node): DeepPartial<Interface> { | ||
const name = node.values[0]; | ||
if (typeof name !== "string") { | ||
throw new Error("Missing mixin name"); | ||
} | ||
|
||
const event: Event[] = []; | ||
const property: Record<string, Partial<Property>> = {}; | ||
const method: Record<string, Partial<Method>> = {}; | ||
|
||
for (const child of node.children) { | ||
switch (child.name) { | ||
case "event": | ||
event.push(handleEvent(child)); | ||
break; | ||
case "property": { | ||
const propName = child.values[0] as string; | ||
const propName = string(child.values[0]); | ||
property[propName] = handleProperty(child); | ||
break; | ||
} | ||
case "method": { | ||
const methodName = string(child.values[0]); | ||
method[methodName] = handleMethod(child); | ||
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. (Fine for now but eventually this will need to change to support overloads) |
||
break; | ||
} | ||
default: | ||
throw new Error(`Unknown node name: ${child.name}`); | ||
} | ||
|
@@ -114,6 +138,7 @@ function handleMixin(node: Node): DeepPartial<Interface> { | |
name, | ||
events: { event }, | ||
properties: { property }, | ||
methods: { method }, | ||
...optionalMember("extends", "string", node.properties?.extends), | ||
} as DeepPartial<Interface>; | ||
} | ||
|
@@ -124,8 +149,8 @@ function handleMixin(node: Node): DeepPartial<Interface> { | |
*/ | ||
function handleEvent(child: Node): Event { | ||
return { | ||
name: child.values[0] as string, | ||
type: child.properties.type as string, | ||
name: string(child.values[0]), | ||
type: string(child.properties.type), | ||
}; | ||
} | ||
|
||
|
@@ -135,13 +160,58 @@ function handleEvent(child: Node): Event { | |
*/ | ||
function handleProperty(child: Node): Partial<Property> { | ||
return { | ||
name: child.values[0] as string, | ||
name: string(child.values[0]), | ||
...optionalMember("exposed", "string", child.properties?.exposed), | ||
...optionalMember("optional", "boolean", child.properties?.optional), | ||
...optionalMember("overrideType", "string", child.properties?.overrideType), | ||
}; | ||
} | ||
|
||
/** | ||
* Handles a child node of type "method" and adds it to the method object. | ||
* @param child The child node to handle. | ||
*/ | ||
function handleMethod(child: Node): Partial<Method> { | ||
const name = string(child.values[0]); | ||
const returnType = child.properties.returns; | ||
|
||
|
||
let typeNode: Node | undefined; | ||
const params: { name: string; type: string }[] = []; | ||
|
||
for (const c of child.children) { | ||
switch (c.name) { | ||
case "type": | ||
if (typeNode) { | ||
throw new Error(`Method "${name}" has multiple type nodes (invalid)`); | ||
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 actually want to allow this for union types, but for now this is ok as we don't have union types right now |
||
} | ||
typeNode = c; | ||
break; | ||
|
||
case "param": | ||
params.push({ | ||
name: string(c.values[0]), | ||
type: string(c.properties.type), | ||
}); | ||
break; | ||
|
||
default: | ||
throw new Error(`Unexpected child "${c.name}" in method "${name}"`); | ||
} | ||
} | ||
|
||
if (!typeNode) { | ||
throw new Error(`Method "${name}" is missing a return type`); | ||
} | ||
|
||
const signature: Method["signature"] = [ | ||
{ | ||
param: params, | ||
...handleTyped(typeNode, returnType), | ||
}, | ||
]; | ||
return { name, signature }; | ||
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 is fine for now but eventually we'll hit a problem when dealing with overloads) |
||
} | ||
|
||
/** | ||
* Collect all file URLs in a directory. | ||
*/ | ||
|
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.
Still have unused parameter
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.
We still use it for the subtype:
https://github.com/Bashamega/TypeScript-DOM-lib-generator/blob/0693d5d3d913903e66e808162f3dcee397681e9c/src/build/patches.ts#L50
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.
Is it needed though? subtype would also be a type, so
isTyped
should be true, and then the second parameter won't be used, right?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 will update it