-
Notifications
You must be signed in to change notification settings - Fork 4
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
Drop old node versions #207
Conversation
lib/Element.js
Outdated
throw new DOMException("Failed to execute 'replaceChild' on 'Node': parameter 2 is not of type 'Node'."); | ||
replaceChildren(newChildren) { | ||
while (this.lastChild) { | ||
this.removeChild(this.lastChild); |
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.
Not sure if this is an issue but removeChild
/ appendChild
methods emits _insert
events. Might result in an active mutation observer triggering multiple times during the execution of this method, rather than once after. Is this expected behavior? Otherwise might be proper to use cheerio internally
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.
Aaah, no, I just moved an implementation that we had in flamingo into tallahassee so that I could remove our local override of the class. I am not super familiar with hos these things are done in tallahassee, but I will look over the implementation!
@@ -40,8 +40,8 @@ module.exports = class WebPage { | |||
method: "GET", | |||
headers: requestHeaders, | |||
}); | |||
assert.equal(resp.status, statusCode, `Unexepected status code. Expected: ${statusCode}. Actual: ${resp.statusCode}`); | |||
assert(resp.headers.get("content-type").match(/text\/html/i), `Unexepected content type. Expected: text/html. Actual: ${resp.headers["content-type"]}`); | |||
assert.equal(resp.status, statusCode, `Unexepected status code. Expected: ${statusCode}. Actual: ${resp.status} (GET ${uri})`); |
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.
👍
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.
👍
TODO: