dperini / nwsapi

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

Regression (2.2.8) surfaced by jsdom: `h1` is not a valid selector #108

Closed FrozenPandaz closed 4 months ago

FrozenPandaz commented 4 months ago

nwsapi@2.2.8 has a regression which causes jsdom to fail saying h1 is not a valid selector.

Run

Repo: https://github.com/FrozenPandaz/jsdom-bug

pnpm i

Error

~/p/t/jsdom-bug (master|✔) $ pnpm test

> jsdom-bug@1.0.0 test /home/jason/projects/triage/jsdom-bug
> jest

(node:14399) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
FAIL  ./test.spec.js
    jsdom
        ✕ should select (4 ms)

    ● jsdom › should select

        SyntaxError: 'h1' is not a valid selector

            2 |     it('should select', () => {
            3 |         const div = document.createElement('div');
        > 4 |         const none = div.querySelector('h1');
                |                          ^
            5 |         expect(none).toBeUndefined();
            6 |     })
            7 | })

            at emit (node_modules/.pnpm/nwsapi@2.2.8/node_modules/nwsapi/src/nwsapi.js:557:17)
            at _querySelectorAll (node_modules/.pnpm/nwsapi@2.2.8/node_modules/nwsapi/src/nwsapi.js:1501:9)
            at Object._querySelector [as first] (node_modules/.pnpm/nwsapi@2.2.8/node_modules/nwsapi/src/nwsapi.js:1412:14)
            at HTMLDivElementImpl.querySelector (node_modules/.pnpm/jsdom@20.0.3/node_modules/jsdom/lib/jsdom/living/nodes/ParentNode-impl.js:69:44)
            at HTMLDivElement.querySelector (node_modules/.pnpm/jsdom@20.0.3/node_modules/jsdom/lib/jsdom/living/generated/Element.js:1094:58)
            at Object.querySelector (test.spec.js:4:26)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.42 s, estimated 1 s
Ran all test suites.
 ELIFECYCLE  Test failed. See above for more details.

Version

~/p/t/jsdom-bug (master|✔) $ pnpm ls nwsapi --depth 3
Legend: production dependency, optional only, dev only

jsdom-bug@1.0.0 /home/jason/projects/triage/jsdom-bug

devDependencies:
jest-environment-jsdom 29.7.0
└─┬ jsdom 20.0.3
└── nwsapi 2.2.8
dperini commented 4 months ago

@FrozenPandaz fixed the regression, the problem was due to a wrong RE trespassing its scope and affecting other areas. Specifically it was the RE used to check an identifier does not start with digits which did completely avoid digits in identifiers affecting all header tags. Thank you for the prompt report.

spamshaker commented 4 months ago

its widely affecting jsdom. In tesing library getByRole('heading') not working as well, when this can be fixed?

spamshaker commented 4 months ago

I can see the issue is in this comparison image

spamshaker commented 4 months ago

temporarly patched the line locally with pnpm patch

      // parse, validate and split possible compound selectors
-      if ((expressions = parsed.match(reValidator)) && expressions.join('') == parsed) {
+      if ((expressions = parsed.match(reValidator)) && expressions.join('') == parsed.replace(/\d/g, '')) {
arjanfrans commented 4 months ago

After updating jsdom I am receiving this error too. Breaking all query selectors with h-tags.

eps1lon commented 4 months ago

Also affecting @testing-library/react and @testing-library/dom: SyntaxError: '*[role~="heading"],h1,h2,h3,h4,h5,h6' is not a valid selector

Downgrading to 2.2.7 fixes it.

dperini commented 4 months ago

@ALL the regression should now be fixed by 2.2.9 Please check and confirm so I can close this issue.

eps1lon commented 4 months ago

Works for us. Thanks for the quick fix!

spamshaker commented 4 months ago

works after update

dperini commented 4 months ago

@spamshaker @eps1lon thank you for the feedback.