Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
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
5 changes: 5 additions & 0 deletions inputfiles/patches/authenticator.kdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
removals {
enum AuthenticatorTransport {
smart-card // WebKit only as of 2023-05
}
}
3 changes: 0 additions & 3 deletions inputfiles/removals/Authenticator.kdl

This file was deleted.

4 changes: 2 additions & 2 deletions src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ async function emitDom() {

const overriddenItems = await readInputJSON("overridingTypes.jsonc");
const addedItems = await readInputJSON("addedTypes.jsonc");
const patches = await readPatches("patches");
const removals = await readPatches("removals");
const patches = await readPatches();
const removals = await readPatches(true);
const comments = await readInputJSON("comments.json");
const documentationFromMDN = await generateDescriptions();
const removedItems = await readInputJSON("removedTypes.jsonc");
Expand Down
86 changes: 59 additions & 27 deletions src/build/patches.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { parse, type Value, type Node } from "kdljs";
import { parse, type Value, type Node, type Document } from "kdljs";
import type {
Enum,
Event,
Expand All @@ -19,26 +19,46 @@
? { [K in keyof T]?: DeepPartial<T[K]> }
: T;

type ParsedType = DeepPartial<WebIdl> & { removals?: DeepPartial<WebIdl> };

interface OverridableMethod extends Omit<Method, "signature"> {
signature: DeepPartial<Signature>[] | Record<number, DeepPartial<Signature>>;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

(this should be restored)

function optionalMember<const T>(prop: string, type: T, value?: Value) {
function optionalMember<const T>(
prop: string,
type: T,
value?: Value | DeepPartial<WebIdl>,
) {
if (value === undefined) {
return {};
}
// Support deep property assignment, e.g. prop = "a.b.c"
const propPath = prop.split(".");
if (typeof value !== type) {
throw new Error(`Expected type ${value} for ${prop}`);
throw new Error(
`Expected type ${type} for ${prop} but got ${typeof value}`,
);
}
return {
[prop]: value as T extends "string"
? string
: T extends "number"
? number
: T extends "boolean"
? boolean
: never,
};
// If value is an object, ensure it is not empty (has at least one key)
if (type === "object" && typeof value === "object" && value !== null) {
if (Object.keys(value as object).length === 0) {
return {};
}
}

// Build the nested object dynamically
let nested: any = value as T extends "string"
? string
: T extends "number"
? number
: T extends "boolean"
? boolean
: never;
for (let i = propPath.length - 1; i >= 0; i--) {
nested = { [propPath[i]]: nested };
}
return nested;
}

function string(arg: unknown): string {
Expand Down Expand Up @@ -79,8 +99,11 @@
/**
* Converts patch files in KDL to match the [types](types.d.ts).
*/
function parseKDL(kdlText: string): DeepPartial<WebIdl> {
const { output, errors } = parse(kdlText);
function parseKDL(kdlText: string | Document): ParsedType {
const { output, errors } =
typeof kdlText === "string"
? parse(kdlText)
: { output: kdlText, errors: [] };

if (errors.length) {
throw new Error("KDL parse errors", { cause: errors });
Expand All @@ -91,8 +114,13 @@
const mixin: Record<string, DeepPartial<Interface>> = {};
const interfaces: Record<string, DeepPartial<Interface>> = {};
const dictionary: Record<string, DeepPartial<Dictionary>> = {};
let removals: DeepPartial<WebIdl> = {};

for (const node of nodes) {
if (node.name === "removals") {
removals = parseKDL(node.children);
continue;
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 the logic change should rather happen in readPatches.

  1. parse(kdl) can happen in readPatches
  2. readPatches then extracts removals from the resulting Document.
  3. Now readPatches has two Document for each file - the normal and the removals.
  4. Pass that to parseKDL() (which maybe a new name, because it doesn't parse anymore).
  5. Return the results as { patches, removalPatches }. (Then the call can happen once rather than twice.)

}
const name = string(node.values[0]);
switch (node.name) {
case "enum":
Expand All @@ -113,10 +141,11 @@
}

return {
enums: { enum: enums },
mixins: { mixin },
interfaces: { interface: interfaces },
dictionaries: { dictionary },
...optionalMember("enums.enum", "object", enums),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this? Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't understand why this change is needed

...optionalMember("mixins.mixin", "object", mixin),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this and below, and generally the dot accessor thing as a whole.

...optionalMember("interfaces.interface", "object", interfaces),
...optionalMember("dictionaries.dictionary", "object", dictionary),
...optionalMember("removals", "object", removals),
};
}

Expand Down Expand Up @@ -375,10 +404,15 @@
return parseKDL(text);
}

/**
* Remove all name fields from the object and its children as we don't want
* the names to be part of the removal.
*/
function removeNamesDeep(obj: unknown): unknown {
if (Array.isArray(obj)) {
return obj.map(removeNamesDeep);
} else if (obj && typeof obj === "object") {
}
if (obj && typeof obj === "object") {
const newObj: { [key: string]: unknown } = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use Record here

for (const [key, value] of Object.entries(obj as Record<string, unknown>)) {
if (key !== "name") {
Expand All @@ -393,19 +427,17 @@
/**
* Read, parse, and merge all KDL files under the input folder.
*/
export default async function readPatches(

Check failure on line 430 in src/build/patches.ts

View workflow job for this annotation

GitHub Actions / test

Replace `⏎··isRemovals?:·boolean,⏎` with `isRemovals?:·boolean`
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be sad to put removal files separately. Would be nice if we can:

// ... somenormal patches ...

// and then removals in the same file
removals {
  enum foo { bar }
}

We can extract all removals node from the result of readPatch and then separately return all extracted removals.

folder: "patches" | "removals",
isRemovals?: boolean,
): Promise<any> {
const patchDirectory = new URL(
`../../inputfiles/${folder}/`,
import.meta.url,
);
const patchDirectory = new URL("../../inputfiles/patches/", import.meta.url);
const fileUrls = await getAllFileURLs(patchDirectory);

const parsedContents = await Promise.all(fileUrls.map(readPatch));
const res = parsedContents.reduce((acc, current) => merge(acc, current), {});
if (folder == "removals") {
return removeNamesDeep(res);
const { removals, ...withoutRemovals } = res;
if (isRemovals) {
return removeNamesDeep(removals);
}
return res;
return withoutRemovals;
}
Loading