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

document.querySelector throws error with bad selector #300

Open
steveworkman opened this issue May 5, 2023 · 3 comments
Open

document.querySelector throws error with bad selector #300

steveworkman opened this issue May 5, 2023 · 3 comments
Assignees

Comments

@steveworkman
Copy link

This library is being run via rollup-plugin-critical -> critical -> inline-critical

When trying to generate an inline stylesheet, the code falls over at https://github.com/bezoerb/inline-critical/blob/main/index.js#L80-L81 with an error of:

 ERROR  'noscript)>[rel="stylesheet"]' 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 _querySelectorAll (node_modules/nwsapi/src/nwsapi.js:1509:36)
  at Object._querySelector [as first] (node_modules/nwsapi/src/nwsapi.js:1415:14)
  at DocumentImpl.querySelector (node_modules/inline-critical/node_modules/jsdom/lib/jsdom/living/nodes/ParentNode-impl.js:69:44)
  at Document.querySelector (node_modules/inline-critical/node_modules/jsdom/lib/jsdom/living/generated/Document.js:1026:58)
  at Dom.querySelector (node_modules/inline-critical/src/dom.js:155:26)
  at inline (node_modules/inline-critical/index.js:85:27)

This isn't the selector being passed in (that would be :not(noscript) > [rel="stylesheet"]) but somewhere around the nwsapi it gets mangled and comes out as a bad selector.

If I comment out these selectors in the code, it works. Adding a selector as an option has no effect as all selectors are evaluated.

@bezoerb
Copy link
Owner

bezoerb commented May 9, 2023

@steveworkman: can you provide a failing testcase for this? i tried to add a test case but the only way i could trigger this kind of error was by providing an invalid selector. Passing :not(noscript) > [rel="stylesheet"] worked as expected.

I also tried this using critical by passing this selector as an option down to inline-critical with the same result. Maybe this error is caused by rollup-plugin-critical itself?

@mpcaples
Copy link

Experiencing the same problem while trying to use critical in our project to inline the above-the-fold css. When inline is true we get the following error:

SyntaxError: 'noscript)>[rel="stylesheet"]' 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 _querySelectorAll (node_modules/nwsapi/src/nwsapi.js:1509:36)
     at Object._querySelector [as first] (node_modules/nwsapi/src/nwsapi.js:1415:14)
     at DocumentImpl.querySelector (node_modules/jsdom/lib/jsdom/living/nodes/ParentNode-impl.js:69:44)
     at Document.querySelector (node_modules/jsdom/lib/jsdom/living/generated/Document.js:1026:58)
     at Dom.querySelector (file://node_modules/inline-critical/src/dom.js:155:26)
     at inline (file:///node_modules/inline-critical/index.js:85:27)

Apparently the code is failing over the same file and lines that @steveworkman referenced above in the inline-critical package, which is a dependency of critical.

@steveworkman
Copy link
Author

I've replicated the issue and it's related to my own project having jsdom@21 as a dependency, which has [email protected] as a dependency. dperini/nwsapi#85 describes the issue and it's related to dperini/nwsapi#83 which is the cause of the issue.

To resolve, we'd need to pin the version of nswapi to 2.2.2 - OR - change that selector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants