Skip to content
This repository has been archived by the owner on Mar 24, 2024. It is now read-only.

Upgrade xmldom #1695

Merged
merged 1 commit into from Aug 24, 2021
Merged

Upgrade xmldom #1695

merged 1 commit into from Aug 24, 2021

Conversation

amacneil
Copy link
Contributor

@amacneil amacneil commented Aug 24, 2021

User-Facing Changes
None

Description
Upgrade xmldom (by switching to @xmldom/xmldom) to fix CVE-2021-32796. See xmldom/xmldom#271 for background on why the package name changed.

Types are now included in the package (xmldom/xmldom#191). This PR adds "lib": ["dom"] to hooks package tsconfig because the previous xmldom types were automatically adding this for us with a <reference lib="dom" /> (xmldom/xmldom#285).

@@ -3,6 +3,7 @@
"include": ["./src/**/*"],
"compilerOptions": {
"rootDir": "./src",
"outDir": "./dist"
"outDir": "./dist",
"lib": ["dom", "es2020"]
Copy link
Contributor Author

@amacneil amacneil Aug 24, 2021

Choose a reason for hiding this comment

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

@defunctzombie
Copy link
Contributor

LGTM

@amacneil amacneil merged commit ccdab87 into main Aug 24, 2021
@amacneil amacneil deleted the adrian/xmldom branch August 24, 2021 05:00
@karfau
Copy link

karfau commented Aug 24, 2021

@amacneil (from the xmldom types perspective), I would really love to understand why the hooks package needed the change in tsconfig.json, since in the file you linked I do not see any reference to xmldom.
What exactly was the error message?

(The file you linked contains a reference to a global document, but that is not something that xmldom would "add for you".)

@defunctzombie
Copy link
Contributor

@karfau The hooks package needed to specify "dom" in the lib config because the code in useVisibilityState (linked in an above comment) references the global "document". For that we need to tell typescript we want to bring in the dom library into our scope.

@karfau
Copy link

karfau commented Aug 24, 2021

Yes, that makes sense. I just don't understand why this change was required as part of this PR/how it is related to updating xmldom.

@defunctzombie
Copy link
Contributor

defunctzombie commented Aug 24, 2021

@karfau The previous xmldom types (@types/xmldom) had <reference lib="dom" />. This changes the typescript compiler configuration to add "dom" lib for any users of those types. The new types don't do this so the end user should add dom to their lib themselves.

@amacneil
Copy link
Contributor Author

Weirdly the new types do still contain that reference, but typescript definitely ignored it when I upgraded the package.

I will try to make a minimal reproduction of this issue today.

@amacneil
Copy link
Contributor Author

Moving discussion to xmldom/xmldom#285

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants