search

dperini / nwsapi

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

v2.2.3 DOMException [SyntaxError]: '.disabled):not(:disabled)' is not a valid selector #81

Closed jdufresne closed 1 year ago

jdufresne commented 1 year ago

The following selector was used successfully in v2.2.2 but raises a syntax error v2.2.3:

[data-bs-toggle="dropdown"]:not(.disabled):not(:disabled)

This selector is used by the Bootstrap library:

https://github.com/twbs/bootstrap/blob/3d84e60d690b9755d29f2a8bb9ca807f484768bd/js/src/dropdown.js#L55

Script to reproduce:

const jsdom = require('jsdom');

const dom = new jsdom.JSDOM(`<!DOCTYPE html><p>Hello world</p>`);
dom.window.document.querySelectorAll('[data-bs-toggle="dropdown"]:not(.disabled):not(:disabled)');
$ node test.js

.../node_modules/nwsapi/src/nwsapi.js:559
        throw err;
        ^
DOMException [SyntaxError]: '.disabled):not(:disabled)' is not a valid selector
    at emit (.../node_modules/nwsapi/src/nwsapi.js:557:17)
    at Object._matches [as match] (.../node_modules/nwsapi/src/nwsapi.js:1400:9)
    at Array.Resolver (eval at compile (.../node_modules/nwsapi/src/nwsapi.js:760:17), <anonymous>:3:105)
    at collect (.../node_modules/nwsapi/src/nwsapi.js:1552:21)
    at Object._querySelectorAll [as select] (.../node_modules/nwsapi/src/nwsapi.js:1509:36)
    at DocumentImpl.querySelectorAll (.../node_modules/jsdom/lib/jsdom/living/nodes/ParentNode-impl.js:78:26)
    at Document.querySelectorAll (.../node_modules/jsdom/lib/jsdom/living/generated/Document.js:1051:58)
    at Object.<anonymous> (.../test.js:4:21)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)

Node.js v18.15.0
flannanl commented 1 year ago

I'm seeing the same issue after moving up to v2.2.3. In my case, I have 3 :not().

.main-nav:not(.header-nav):not(.settings-nav):not(.other-nav)

paf02 commented 1 year ago

We were having the same issue, it is saying that the selector is not valid, rolling back to the 2.2.2 version in the package-lock.json works fine for us.

Screenshot 2023-04-10 at 4 00 10 PM

ankursehdev commented 1 year ago

Having same issue via JsDOM.

dperini commented 1 year ago

@jdufresne & all The issue should be resolved in release 2.2.4. Please review and confirm that everything works. Thank you for submitting a reproducible code showing the issue.

jdufresne commented 1 year ago

@dperini v2.2.4 worked for me. Thanks for the quick fix and response!

Schnueggel commented 1 year ago

For me 2.2.4 does not work. I use chakra-ui and they use this selector "button:not(:disabled):not([disabled])". I used "resolutions" in package.json to force 2.2.4. My tests still fail. With 2.2.2 they work.

dperini commented 1 year ago

@Schnueggel for me the above selector you tested works. It does not throw errors and seems to be working as expected. To ensure it is working, I successfully checked the following similar selectors permutations:

document.querySelectorAll('button:not(.disabled)');
document.querySelectorAll('button:not(:disabled)');
document.querySelectorAll('button:not([disabled])');

document.querySelectorAll('button:not(.disabled):not(:disabled)');
document.querySelectorAll('button:not(.disabled):not([disabled])');
document.querySelectorAll('button:not(:disabled):not([disabled])');

document.querySelectorAll('button:not(.disabled):not(:disabled):not([disabled])');

all of them are passing without any parsing error, so I can't say what's wrong on your environment. I suggest you check again ensuring your cache is fresh and that you also test with with a different browser.

If you can post a code with the reproducible failing case I will be happy to test it and find a solution.

AndresREIB commented 1 year ago

I also use chraka-ui and 2.2.4 also does not work for me. This is what I get: SyntaxError: ':disabled):not([disabled]' is not a valid selector I get this when running component tests with jest. If I override to 2.2.2, it all works fine.

dperini commented 1 year ago

@Schnueggel & @AndresREIB as an extra tip to the testing task, notice that there is an equivalent syntax of the ":not()" pseudo-class:

The selector ":not( s1 ):not( s2 ):not( s3 )" is equivalent to the selector ":not( s1, s2, s3 )"

so the following two queries will return the same results::

document.querySelectorAll(':not(.disabled):not(:disabled):not([disabled])');
document.querySelectorAll(':not(.disabled,:disabled,[disabled])');

You can type these queries in the console of a page where the "nwsapi" has been loaded.

Schnueggel commented 1 year ago

newest Chrome supports this syntax. The Problem is the jest testing. The jsdom lib uses nwsapi. I will try to look a bit deeper into things.

Schnueggel commented 1 year ago

there comes a list of selectors including this one :not(:disabled):not([disabled])

This will compile the following resolver

(function anonymous(s) {
  return function Resolver(c, f, x, r) {
    var e,
      n,
      o,
      j = r.length - 1,
      k = -1
    while ((e = c[++k])) {
      if (!s.match(':disabled):not([disabled]', e)) {
        r[++j] = c[k]
        continue
      }
    }
    return r
  }
})

The match function failes then of course

giorgosera commented 1 year ago

I have exactly the same issue. I'm also using Chakra and React testing library. I had to use overrides like this

  "overrides": {
    "nwsapi": "2.2.2"
  },

This resolves the issue but I wonder how this can be resolved permanently.

mikhin commented 1 year ago

Confirm the problem with Chakra and React testing library.

dbashford commented 1 year ago

Also confirming, and for anyone who struggles with it, for overrides to work properly you need npm 8.6.0.

zgrybus commented 1 year ago

I have exactly the same issue. I'm also using Chakra and React testing library. I had to use overrides like this

  "overrides": {
    "nwsapi": "2.2.2"
  },

This resolves the issue but I wonder how this can be resolved permanently.

It helped, thanks. However, I'm looking forward permanent solution.

dperini commented 1 year ago

@jdufresne and all developers in this thread/issue, in commit e60b058 I am trying to fix the :not() pseudo-class bug Could you please test your specific case is fixed by these changes ? In cases where this is not fixed please help by reporting the selector here.

Thank you all for the heads-up, for the suggestions and for the help in solving this bug.

GabOsaz commented 1 year ago

I have exactly the same issue. I'm also using Chakra and React testing library. I had to use overrides like this

  "overrides": {
    "nwsapi": "2.2.2"
  },

This resolves the issue but I wonder how this can be resolved permanently.

It helped, thanks. However, I'm looking forward permanent solution.

Worked for me!

ttcode10 commented 1 year ago

I have exactly the same issue. I'm also using Chakra and React testing library. I had to use overrides like this

  "overrides": {
    "nwsapi": "2.2.2"
  },

This resolves the issue but I wonder how this can be resolved permanently.

Thanks for the solution, but where should we put this script? Thanks

m-nouman-ashraf commented 1 year ago

I have exactly the same issue. I'm also using Chakra and React testing library. I had to use overrides like this

  "overrides": {
    "nwsapi": "2.2.2"
  },

This resolves the issue but I wonder how this can be resolved permanently.

Thanks for the solution, but where should we put this script? Thanks

In the package.json file below the "devDependencies"

jsvossen commented 1 year ago

@jdufresne and all developers in this thread/issue, in commit e60b058 I am trying to fix the :not() pseudo-class bug Could you please test your specific case is fixed by these changes ? In cases where this is not fixed please help by reporting the selector here.

Thank you all for the heads-up, for the suggestions and for the help in solving this bug.

@dperini the fix in your commit appears to be working, though I don't see it in v2.2.4. Will it be released in v2.2.5 soon?

james4388 commented 1 year ago

for yarn

"resolutions": {
    "**/nwsapi": "2.2.2"
  },
dperini commented 1 year ago

@jsvossen thank you for testing out the latest changes and help ensure this is fixed. Though you are the only one that answered to my request for help.

So I will have to wait for a couple more confirmation from other developers having issues with other selectors before pushing a new 2.2.5 release on npm.

I really appreciated your input, thank you !

barrymichaeldoyle commented 1 year ago

I'm also having this issue with Chakra in my Jest testing (using RTL)...

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.

bfsgr commented 1 year ago

@jdufresne and all developers in this thread/issue, in commit e60b058 I am trying to fix the :not() pseudo-class bug Could you please test your specific case is fixed by these changes ? In cases where this is not fixed please help by reporting the selector here.

Thank you all for the heads-up, for the suggestions and for the help in solving this bug.

I tested this change and so far it seams to fix the problems I was having with Chakra and RTL, thanks!!

yarinsa commented 1 year ago

Same for us using @chakra-ui. Downgrading to 2.2.2 worked