Skip to content
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

Changed the type of the "node" in hooks to be as specific as possible #1031

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

reduckted
Copy link
Contributor

Fixes #1029.

Prior to the TypeScript type declarations being provided by this package, the @types/dompurify package defined all hooks as taking an Element as the first parameter. That was incorrect because some hooks can be given a Node or DocumentFragment. When the type declarations were moved into this repository, the hook's first parameter type was changed to Node. This change was overzealous because some hooks are guaranteed to only be given an Element or DocumentFragment.

To account for those hooks, this pull request adds two additional hook types - ElementHook and DocumentFragmentHook (plus Hook has been renamed to NodeHook to disambiguate it).

Hook Type of currentNode
beforeSanitizeElements Node
afterSanitizeElements Node
beforeSanitizeShadowDOM DocumentFragment
uponSanitizeShadowNode Node
afterSanitizeShadowDOM DocumentFragment
beforeSanitizeAttributes Element
afterSanitizeAttributes Element
uponSanitizeElement Node
uponSanitizeAttribute Element

To make this as type-safe as possible internally, I've changed the hooks to be stored in an object with properties named after each hook rather than just in an object that keys a string to an array of functions. The _executeHooks function now takes in the array of hooks to execute rather than the name of the hook. This allows the "node" and "data" parameters for the hook to have type-checking applied at compile time to ensure that, for example, you don't call _executeHooks with a Node when the hooks you are executing require an Element.

@cure53
Copy link
Owner

cure53 commented Nov 20, 2024

Awesome, thank you!

@cure53 cure53 merged commit e972e6e into cure53:main Nov 22, 2024
8 checks passed
@reduckted reduckted deleted the feature/hook-parameter-types branch November 22, 2024 22:14
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.

Incorrect type for addHook callback currentNode argument
2 participants