KDL support for removals #2250

Bashamega · 2025-11-07T07:41:10Z

#2053

Added support for removing items using KDL

github-actions · 2025-11-07T07:41:27Z

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.

Bashamega · 2025-11-24T03:58:34Z

What do you think @saschanaz

saschanaz · 2025-11-27T09:46:51Z

A bit busy... hopefully this weekend.

Bashamega · 2025-11-27T10:33:37Z

No problem

src/build/patches.ts

saschanaz · 2025-11-30T15:28:20Z

src/build/patches.ts

 */
-export default async function readPatches(): Promise<any> {
-  const patchDirectory = new URL("../../inputfiles/patches/", import.meta.url);
+export default async function readPatches(


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.

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

saschanaz · 2025-11-30T17:22:59Z

src/build/patches.ts

+    if (node.name === "removals") {
+      removals = parseKDL(node.children);
+      continue;


I think the logic change should rather happen in readPatches.

parse(kdl) can happen in readPatches

readPatches then extracts removals from the resulting Document.

Now readPatches has two Document for each file - the normal and the removals.

Pass that to parseKDL() (which maybe a new name, because it doesn't parse anymore).

Return the results as { patches, removalPatches }. (Then the call can happen once rather than twice.)

saschanaz · 2025-12-01T08:01:32Z

src/build/patches.ts

 */
-function parseKDL(kdlText: string): DeepPartial<WebIdl> {
-  const { output, errors } = parse(kdlText);
+function convertKDLNodes(nodes: Node[] | Document): DeepPartial<WebIdl> {


Hmm? Document is equivalent to Node[], this shouldn't be needed.

saschanaz · 2025-12-01T08:01:51Z

src/build/patches.ts

-    mixins: { mixin },
-    interfaces: { interface: interfaces },
-    dictionaries: { dictionary },
+    ...optionalMember("enums.enum", "object", enums),


Do we need this? Why?

I still don't understand why this change is needed

saschanaz · 2025-12-01T08:03:08Z

src/build/patches.ts

+    const newObj: { [key: string]: unknown } = {};
+    for (const [key, value] of Object.entries(obj as Record<string, unknown>)) {
+      if (key !== "name") {
+        newObj[key] = removeNamesDeep(value);


Do you have a plan how to make nulls work to remove other things than enum values?

Bashamega · 2025-12-01T09:19:20Z

I have fixed it, and added support for dictionaries @saschanaz

saschanaz · 2025-12-01T09:23:49Z

src/build/patches.ts

-    mixins: { mixin },
-    interfaces: { interface: interfaces },
-    dictionaries: { dictionary },
+    ...optionalMember("enums.enum", "object", enums),


I still don't understand why this change is needed

saschanaz · 2025-12-01T09:24:21Z

src/build/patches.ts

+  // Accept either Document or array of nodes
+  const actualNodes: Node[] = Array.isArray(nodes)
+    ? nodes
+    : nodes;


nodes or nodes is just nodes

src/build/patches.ts

saschanaz · 2025-12-01T09:26:58Z

src/build/patches.ts

+    return result.length === 0 ? null : result;
+  }
+  if (obj && typeof obj === "object") {
+    const newObj: { [key: string]: unknown } = {};


Can use Record here

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

Bashamega · 2025-12-01T09:39:30Z

Done @saschanaz

saschanaz · 2025-12-01T09:56:25Z

src/build/patches.ts

+    dictionaries: {
+      dictionary,
+    },
+    ...optionalMember("mixins.mixin", "object", mixin),


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

Bashamega · 2025-12-01T10:46:30Z

Is it better now @saschanaz

saschanaz · 2025-12-01T19:12:45Z

src/build/patches.ts

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


(this should be restored)

saschanaz · 2025-12-01T21:08:10Z

src/build/patches.ts

+  // Only because we don't use them
+  removalPatches.mixins = undefined;
+  removalPatches.interfaces = undefined;


Does this matter - if we omit this does the test fail?

Yes without it the tests fail

Ah because interface: {} becomes interface: null. Hmmmmm I think this may be a footgun later when some patches become redundant and are removed...

src/build/patches.ts

saschanaz · 2025-12-01T21:09:51Z

src/build/patches.ts

+ * Recursively remove all 'name' fields from the object and its children, and
+ * replace any empty objects ({} or []) with null.
+ */
+function sanitizeRemovals(obj: unknown): unknown {


convertForRemovals? It's not really sanitizing - normalizing random user input, but rather converting into a different format.

src/build/patches.ts

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

Bashamega · 2025-12-02T09:20:12Z

Thanks @saschanaz
Is there anything else left?

saschanaz · 2025-12-02T09:28:01Z

LGTM, thanks!

github-actions · 2025-12-02T09:28:19Z

Merging because @saschanaz is a code-owner of all the changes - thanks!

KDL support for removals

743c7a3

saschanaz reviewed Nov 30, 2025

View reviewed changes

Bashamega and others added 6 commits November 30, 2025 18:05

Update src/build/patches.ts

8b611f7

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

Update src/build/patches.ts

5184d40

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

Support removals

94ee9a9

Refactor readPatches to separate removals from the main result object

d656a14

Clean

117e76b

-

790f3ef

saschanaz reviewed Nov 30, 2025

View reviewed changes

Bashamega added 3 commits December 1, 2025 07:02

Refactor

8ffb44c

-

b61ee71

-

184eb7e

saschanaz reviewed Dec 1, 2025

View reviewed changes

Bashamega added 2 commits December 1, 2025 11:00

-

6b779dd

-

6e76de9

saschanaz reviewed Dec 1, 2025

View reviewed changes

Bashamega and others added 3 commits December 1, 2025 11:33

Update src/build/patches.ts

8350852

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

-

d2ce62e

-

a92bc94

saschanaz reviewed Dec 1, 2025

View reviewed changes

-

26e7af8

saschanaz reviewed Dec 1, 2025

View reviewed changes

Update src/build/patches.ts

162b92c

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

Bashamega and others added 4 commits December 2, 2025 07:58

Update src/build/patches.ts

e227deb

Co-authored-by: Kagami Sascha Rosylight <[email protected]>

-

2a4aa6a

-

54ec2f5

hotfix

fdd8294

saschanaz added 2 commits December 2, 2025 10:23

undefinedifempty

729d58a

webauthn

894101d

github-actions bot merged commit 9969062 into microsoft:main Dec 2, 2025
5 checks passed

saschanaz deleted the removals branch December 2, 2025 09:28

KDL support for removals #2250

KDL support for removals #2250

Conversation

Bashamega commented Nov 7, 2025

Uh oh!

github-actions bot commented Nov 7, 2025

Uh oh!

Bashamega commented Nov 24, 2025

Uh oh!

saschanaz commented Nov 27, 2025

Uh oh!

Bashamega commented Nov 27, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bashamega commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bashamega commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bashamega commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saschanaz Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Bashamega commented Dec 2, 2025

Uh oh!

saschanaz commented Dec 2, 2025

Uh oh!

Uh oh!

github-actions bot commented Dec 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

saschanaz Dec 1, 2025 •

edited

Loading