Closed ewlsh closed 1 year ago
dperini
commented
1 year ago @ewlsh great finding ... these kind of bugs might be affecting other selectors too. I will have this fixed asap together with others I am currently working on. Thank you for your help and for reporting the error.
dperini
commented
1 year ago @ewlsh the issue should be resolved by the above commit soo I am closing this. Feel free to reopen in case you still have problems, remember to send a reproducible test case.
jas-chen
commented
1 year ago Hi @dperini I still encounter this issue after upgrading to v2.2.3. Here is the reproducible test case, the issue is caused by the element property on the custom element.
https://github.com/jas-chen/nwsapi-issue-73
dperini
commented
1 year ago @jas-chen Thank you for reporting this issue. I will have a deeper look to this. While at it, can you also repeat the same test using the latest 2.2.4 release ? Release 2.2.3 was a failure and I will probably remove it from npm in a few days.
dperini
commented
1 year ago @jas-chen & @ewlsh here is a simplified test case as an attempt to triage this bug in the right repository, it is the same test case as posted above, I just got rid of React and Jest dependencies and left the simplest native JavaScript code to reproduce this bug. Here is the HTML/JavaScript code with which I performed my test:
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>customElements and nwsapi</title>
<script src="src/nwsapi.js"></script>
<script>
window.onload = () => {
class MyElement extends HTMLElement {
static properties = {
element: {},
};
render() {
return this.element;
}
}
customElements.define("my-element", MyElement);
const style = document.createElement("style");
style.innerHTML = `
body::before {
content: '';
}`;
document.head.appendChild(style);
document.body.innerHTML = '<my-element element="hello"></my-element>';
const myElement = document.querySelector("my-element");
// this replaces the "expect" line in the original test
console.log(myElement.attributes['element'].value);
console.log(window.getComputedStyle(myElement));
// these are additional queries to ensure nwsapi works
console.log(document.querySelector("my-element"));
console.log(document.querySelector("my-element[element]").attributes);
console.log(document.querySelector("my-element[element]").getAttribute('element'));
};
</script>
</head>
<body>
</body>
</html>Please copy the above code and paste it in the console and check if the results are correct. If that is the case, I guess we should start looking for the issue in another place.
Thank you for the help with this.
jas-chen
commented
1 year ago @dperini
can you also repeat the same test using the latest 2.2.4 release ?
Yes, the issue is still there after upgrading to 2.2.4.
Please copy the above code and paste it in the console and check if the results are correct.
The result is not correct, myElement.element should have value 'hello', also the error TypeError: Cannot read properties of undefined (reading 'toLowerCase') is not thrown.
Here is the code that I consider to be correct (note: the API of my-element is changed, the attribute element is replaced with the element property).
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>customElements and nwsapi</title>
<script src="src/nwsapi.js"></script>
<script>
window.onload = () => {
class MyElement extends HTMLElement {
get element() {
return "hello";
}
render() {
return this.element;
}
}
customElements.define("my-element", MyElement);
const style = document.createElement("style");
style.innerHTML = `
body::before {
content: '';
}`;
document.head.appendChild(style);
document.body.innerHTML = '<my-element></my-element>';
const myElement = document.querySelector("my-element");
if (myElement.element !== "hello") {
throw new Error(`myElement.element should have value 'hello'`);
}
// should throw TypeError: Cannot read properties of undefined (reading 'toLowerCase')
console.log(window.getComputedStyle(myElement));
};
</script>
</head>
<body>
</body>
</html>@jas-chen & @ewlsh
the results of the tests executed with "nwsapi" and native qSA are the same, that tells me "nwsapi" is ok but I don't know the code you are using nor am I familiar with your environment so there might be quirks I did not consider.
If you can setup a minimal native JavaScript/DOM code showing different behavior compared to that of "nwsapi", then I can dig further in the issue and try to fix my code else I should give up and only rely on your input.
The scope of "nwsapi" is mimic and replicate the functionality of the native browsers qSA, in browsers and headless environments like "jsdom".
Looking forward for a reproducible test case, without dependencies, showing the issue in "nwsapi". Thank you for your help and availability.
dperini
commented
1 year ago @jas-chen & @ewlsh I forgot to tell that in case we do not discover the reason of the issue or if we are unable to fix it, I am open to consider adding that small extra check that @ewlsh mentioned in his first report, thus changing:
e.element&&e.type.toLowerCase()with an extra check as suggested:
e.element&&e.type&&e.type.toLowerCase()ensuring first that this change doesn't affect other functionalities or produces other issues.
jas-chen
commented
1 year ago @dperini Sorry I didn't test the HTML code in the browser, it seems that only breaks in jsdom.
I made another code that should produce the issue in the browser (tested with Chrome).
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<script src="https://unpkg.com/nwsapi@2.2.4/src/nwsapi.js"></script>
</head>
<body>
<script>
class MyElement extends HTMLElement {
get element() {
return "hello";
}
render() {
return this.element;
}
}
customElements.define("my-element", MyElement);
document.body.innerHTML = '<my-element></my-element>';
function match() {
return document.querySelector("my-element").matches('my-element::after');
}
// this works
console.log(match());
NW.Dom.install();
// throws `TypeError: Cannot read properties of undefined (reading 'toLowerCase')`
console.log(match());
</script>
</body>
</html>@dperini please correct me if I am wrong, but I don't see this issue fixed?
Hi š I'm working on upgrading our codebase from Jest 26 to Jest 29 and alongside that we're upping our
jsdomversion and thusnwsapi. I've been struggling with this very odd error in some components and eventually tracked it down to the codenwsapiis generating. What seems to be happening is whenever a component contains duet date picker*ByRoleselectors from React Testing Library start to fail. It seems to occur when jsdom evaluates a component within the duet date picker.It seems that with this code generated by
nwsapi,e.element&&e.type.toLowerCase()introduced heree.typeisundefinedfor these duet components and thus the generated code throws an error. I changed this to bee.element&&e.type&&e.type.toLowerCase()locally and my tests are now passing again.Duet does use custom web elements, but I'm not sure if that is relevant. Additionally it uses more complex selectors than most of our components, so could also be a factor.
The error:
Code generated before change:
Code generated after change: