bezoerb / inline-critical

Inline critical path CSS and async load existing stylesheets
Other
115 stars 15 forks source link

document.querySelector throws error with bad selector #300

Open steveworkman opened 1 year ago

steveworkman commented 1 year ago

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

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

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

I've replicated the issue and it's related to my own project having jsdom@21 as a dependency, which has nswapi@2.2.4 as a dependency. https://github.com/dperini/nwsapi/issues/85 describes the issue and it's related to https://github.com/dperini/nwsapi/issues/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