Skip to content

Make options required in MutationObserver.observe() - #2578

Merged
github-actions[bot] merged 2 commits into
microsoft:mainfrom
MartinFerret:fix/mutationobserver-observe-options-required
Nov 3, 2026
Merged

github-actions[bot] merged 2 commits into
microsoft:mainfrom
MartinFerret:fix/mutationobserver-observe-options-required

Conversation

@MartinFerret

@MartinFerret Martin Ferret (MartinFerret) commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Part of #2566

observe() throws a TypeError whenever none of childList, attributes or characterData ends up true, so calling observe(target) without options always throws (covered by WPT in dom/nodes/MutationObserver-sanity.html). As discussed in the issue, this makes the options parameter required, which flags that mistake at compile time without affecting code that works at runtime.

Since the KDL patch format didn't support optional on method parameters yet, this PR also adds that to handleParam in src/build/patches.ts (one line), then uses it in inputfiles/patches/dom.kdl. If you'd prefer this override in overridingTypes.jsonc instead, I'm happy to move it.

Note: this only covers calling observe(target) without options. Options objects that don't meet the minimum requirements, such as {} or { subtree: true }, are still accepted by the type and throw at runtime, so I'm not closing the issue with this PR.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the PR!

This section of the codebase is owned by Kagami Sascha Rosylight (@saschanaz) - if they write a comment saying "LGTM" then it will be merged.

@MartinFerret

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@saschanaz

Copy link
Copy Markdown
Contributor

Thanks! LGTM.

@github-actions
github-actions Bot merged commit b720442 into microsoft:main Nov 3, 2026
6 checks passed
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Merging because Kagami Sascha Rosylight (@saschanaz) is a code-owner of all the changes - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants