dperini / nwsapi

Fast CSS Selectors API Engine
MIT License
103 stars 35 forks source link

Not a valid selector #89

Open jsmolka opened 1 year ago

jsmolka commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

Confirmed 2.2.5 fixes the issue for me.

dperini commented 1 year ago

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

ben-reitz commented 1 year ago

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

Piliuta commented 1 year ago

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 commented 1 year ago

v2.2.5 works for me aswell, thanks.

TomONeill commented 1 year ago

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 commented 1 year ago

@Piliuta @TomONeill @ALL commit 01216f85b9989ce0f1d5b0ee5ce28bb88ea8aaa3 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 commented 1 year ago

Hello @dperini It looks like there are still issues/regressions with pseudo selector detection (even with commit https://github.com/dperini/nwsapi/commit/01216f85b9989ce0f1d5b0ee5ce28bb88ea8aaa3) . 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 commented 1 year ago

@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 commented 9 months ago

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