Skip to content

Adds a rule to make sure that optional params can't come before required #2274

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.

Sign up for GitHub

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

Closed
Bashamega wants to merge 1 commit into from
+16 −18
Closed

Adds a rule to make sure that optional params can't come before required #2274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 0 additions & 16 deletions inputfiles/overridingTypes.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,22 +288,6 @@
},
"interfaces": {
"interface": {
"GPUPipelineError": {
// Web IDL optional argument can be followed by a required argument, but not in TS
// TODO: Maybe do this in the emitter?
"constructor": {
"signature": {
"0": {
"param": [
{
"name": "message",
"optional": false
Copy link
Contributor

@saschanaz saschanaz Dec 7, 2025

Choose a reason for hiding this comment

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

This was ok because it was one-off, but it's not great to automate this nonideal workaround.

Given the IDL parameters of "required, optional, and then required" pattern, both form of the function call should be allowed:

func(required);
func(required, undefined, required);

This can be only done with overloads in current TS. I'm not sure it's worth right now given it's very rare to have this problem.

}
]
}
}
}
},
"HTMLElement": {
"properties": {
"property": {
Expand Down
18 changes: 16 additions & 2 deletions src/build/widlprocess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,29 @@ function convertCallbackFunctions(
};
}

function convertArgument(arg: webidl2.Argument): Browser.Param {
function hasRequiredArgumentAfter(arr: webidl2.Argument[], i: number): boolean {
Copy link
Contributor

@saschanaz saschanaz Dec 7, 2025

Choose a reason for hiding this comment

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

Also I think this is a wrong layer to solve the problem. widlprocess should be about conversion to the format that the build script understands, and not about changing anything. That should be part of the emitter, as the comment above says.

for (let j = i + 1; j < arr.length; j++) {
if (!arr[j].optional && !arr[j].variadic) {
return true;
}
}
return false;
}

function convertArgument(
arg: webidl2.Argument,
i: number,
arr: webidl2.Argument[],
): Browser.Param {
const idlType = convertIdlType(arg.idlType);
const required = hasRequiredArgumentAfter(arr, i);
if (hasExtAttr(arg.extAttrs, "LegacyNullToEmptyString")) {
idlType.nullable = true;
}
return {
name: arg.name,
...idlType,
optional: arg.optional,
optional: !required && arg.optional,
variadic: arg.variadic,
allowShared: hasExtAttr(arg.extAttrs, "AllowShared"),
};
Expand Down
Loading