-
Notifications
You must be signed in to change notification settings - Fork 9
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
Rework of DOM shim (using parse5) to allow for element properties in the future #178
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will try and give this a more thorough testing soon, but just a couple quick observations:
- Since there are no dependency changes, package-lock.json should probably not be changing
- It looks like the tests failed only because of coverage thresholds, and so I'm fine bumping down that as needed (in this case to 85% would be ok)
Ah, my bad. I must have had it accidentally checked on Sourcetree, just reset the branch and pushed just the relevant files
Yes, I believe on my initial commit I was at around 91+% on function coverage but it looks like it dipped to 89.28% with some of the additional changes. Yes, if we can temporarily drop the threshold to 85% that would be great |
…ntent - other relevant changes as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, coming out of our chats in #177, here's the state of things
- We will land feature/issue 177 Replaced deprecated method getInnerHTML with getHTML #171 first
- preserve
getHTML
in the DOM shim and make sure no other APIs are being removed (and we should probably add some test cases here, which I can help with) - I am OK to let the mode fix come in as part of this PR, but let's make sure by adding a test case - support configurable
shadowrootmode
attribute for<template>
tags #65 - Looks like this PR would also help resolve or invalidate verify / ensure proper serialization of shadow roots excluding closed shadow roots from
getInnerHTML
#16 ? - Sanity test in Greenwood (I will help with this once this gets rebased after feature/issue 177 Replaced deprecated method getInnerHTML with getHTML #171 lands)
Related Issue
First step to resolving issue 170
Summary of Changes
dom-shim.js:
wcc.js