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

Not a valid selector #89

Open
jsmolka opened this issue May 19, 2023 · 13 comments
Open

Not a valid selector #89

jsmolka opened this issue May 19, 2023 · 13 comments

Comments

@jsmolka
Copy link

jsmolka commented May 19, 2023

I am using jsdom, which uses this library as a dependency. Running document.querySelector(':is(code, pre)[class^="language-"]') raises a syntax error, which did not happen in earlier versions.

The problem must be caused by a change between version 2.2.2 (worked) and 2.2.3 (does not work).

Forcing an older version of this library in the package.json fixes the issue.

"overrides": {
  "nwsapi": "2.2.2"
}
@tomjnunes
Copy link

Similar issue, but with 2.2.4. The selector *:not(.nav__toggle):not([type="checkbox"]) worked fine in 2.2.2. With 2.2.4 it is being parsed to .nav__toggle):not([type="checkbox"].

@ben-reitz
Copy link

We've seen the same issue - we've had to force resolution to 2.2.2 as we're getting the following error many times in our unit test suite through jsdom:

SyntaxError: 'slot):not([inert]' is not a valid selector
 ❯ emit node_modules/nwsapi/src/nwsapi.js:557:17
 ❯ Object._matches [as match] node_modules/nwsapi/src/nwsapi.js:1400:9
 ❯ Array.Resolver node_modules/nwsapi/src/nwsapi.js:760:17
 ❯ collect node_modules/nwsapi/src/nwsapi.js:1552:21
 ❯ Object._querySelectorAll [as select] node_modules/nwsapi/src/nwsapi.js:1509:36
 ❯ HTMLDivElementImpl.querySelectorAll node_modules/jsdom/lib/jsdom/living/nodes/ParentNode-impl.js:78:26
 ❯ HTMLDivElement.querySelectorAll node_modules/jsdom/lib/jsdom/living/generated/Element.js:1119:58

@dperini
Copy link
Owner

dperini commented May 25, 2023

@ALL
please test if the new changes to the Regular Expression works in your cases.
This will be in 2.2.5 as soon as everybody confirm their case is resolved by this change.

@EvHaus
Copy link

EvHaus commented May 29, 2023

Confirmed 2.2.5 fixes the issue for me.

@dperini
Copy link
Owner

dperini commented May 29, 2023

@EvHaus
thank you for testing and reporting,
I couldn't do without testers, much appreciated.

@ben-reitz
Copy link

Can confirm this 2.2.5 fixes the issue in my app too 👍🏼 Thanks @dperini 😄

@Piliuta
Copy link

Piliuta commented May 30, 2023

v2.2.5 fails for me with this selector:

dom.querySelectorAll(':is(div, p, a, h1, h2, h3, h4, h5, h6, ul, ol, li, blockquote):not(:empty) + :is(br, p:empty)')
SyntaxError: 'div, p, a, h1, h2, h3, h4, h5, h6, ul, ol, li, blockquote):not(:empty)+:is(br,p:empty' is not a valid selector

but works fine on v2.2.4

@jsmolka
Copy link
Author

jsmolka commented Jun 8, 2023

v2.2.5 works for me aswell, thanks.

@TomONeill
Copy link

v2.2.5 fails for me with this selector:

dom.querySelectorAll(':is(div, p, a, h1, h2, h3, h4, h5, h6, ul, ol, li, blockquote):not(:empty) + :is(br, p:empty)')
SyntaxError: 'div, p, a, h1, h2, h3, h4, h5, h6, ul, ol, li, blockquote):not(:empty)+:is(br,p:empty' is not a valid selector

but works fine on v2.2.4

Doesn't work for me either on 2.2.5 with selector:
*:is(input, textarea, select):not([hidden]):not([type="hidden"]):not(:disabled)

@dperini
Copy link
Owner

dperini commented Jun 18, 2023

@Piliuta @TomONeill @ALL
commit 01216f8 should have fixed the failures seen above and maybe others.
The Regular Expression in charge of resolving the :is(), :not() and :has() pseudo-classes selectors was more complex than I expected and the w3c changes about multiple compound selectors accepted as arguments made it more complex.
Anyway it is much improved now and I will fix remaining edge cases if any.
Thank you for your patience and testing.

@LucasLefevre
Copy link

LucasLefevre commented Jun 21, 2023

Hello @dperini
It looks like there are still issues/regressions with pseudo selector detection (even with commit 01216f8) .
Specifically with namespaced xml query selectors.

<root xmlns:s="http://example.com"> 
      <s:a></s:a>
</root>

doc.querySelectorAll("s:a") throws with SyntaxError: unknown pseudo-class selector ':a'

This used to work in prior versions (tested with 2.2.2 2.2.0) even if I know support for XML namespaced tag names selectors is only partial.

EDIT: tested with 2.2.0, not 2.2.2

@dperini
Copy link
Owner

dperini commented Jun 28, 2023

@LucasLefevre
the minimal test in "nwsapi/test/xml-test.html" seems to be working correctly as the other tests in the same folder do.
Replacing the elements in "nwsapi/test/xml-test.php" with your suggested XML tags does not work, and it doesn't work using the native QSAPI either. So I guess something is wrong in your tests. I may be wrong though... So please, use the "xml-test.html" in the test folder as a template and replace the elements names with your tags and see where the problem is.

Also the message "unknown pseudo-class selector ':a'" is misleading because this has nothing to do with "pseudo-class" selectors". So I have to fix the misleading error thrown there, but it seems that XML is not a problem currently.

@jeripeierSBB
Copy link

The selector ":is(button, [href], input, select, textarea, details, summary, [tabindex]):not([disabled],[tabindex="-1"],:disabled)" is currently causing an exception.
Could this maybe fixed too? @dperini

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

No branches or pull requests

9 participants