jhildenbiddle / css-vars-ponyfill

Client-side support for CSS custom properties (aka "CSS variables") in legacy and modern browsers
https://jhildenbiddle.github.io/css-vars-ponyfill
MIT License
1.46k stars 64 forks source link

'[object NodeList]' is not a valid argument for 'Function.prototype.apply' #153

Closed bbuhler closed 3 years ago

bbuhler commented 3 years ago

Hello (:

First of all thank you for your great work!

In our Sentry we see an exception connected to your library so I am posting it here:

'[object NodeList]' is not a valid argument for 'Function.prototype.apply' (evaluating 'a.rootElement.querySelectorAll('[data-cssvars="out"]')')
second argument to Function.prototype.apply must be an array (https://cdnjs.cloudflare.com/ajax/libs/css-vars-ponyfill/2.4.3/css-vars-ponyfill.min.js#8)

It appears mainly on Firefox (24) and (Mobile) Safari.

Cheers, Ben

tiperes commented 3 years ago

Same here when trying to use with wkhtmltopdf... :(

tiperes commented 3 years ago

I have overcome the issue by wrapping all query selectors with Array.prototype.slice.call. Example: Array.apply(null,Array.prototype.slice.call(o.rootElement.querySelectorAll('[data-cssvars]:not([data-cssvars="out"])')))

jhildenbiddle commented 3 years ago

Based on query selector in @bbuhler's error message and @tiperes' proposed fix, it looks like you both might be experiencing the same issue but at different points in the code:

@bbuhler:

https://github.com/jhildenbiddle/css-vars-ponyfill/blob/f37215c2c072b6a5695f08bf5d91898cb3640216/src/index.js#L278

@tiperes:

https://github.com/jhildenbiddle/css-vars-ponyfill/blob/f37215c2c072b6a5695f08bf5d91898cb3640216/src/index.js#L244

This feels like a similar to #144, which is to say the code above works fine but perhaps there is some app- or framework-specific condition that can cause the rootElement or the node list returned from the querySelectorAll() method to become invalid while the pony fill is doing its work. Hard to say without doing a deeper div into each app.

I have created a branch with a possible fix: https://github.com/jhildenbiddle/css-vars-ponyfill/tree/fix-153. I just switched all nodelist-to-array conversions from Array.apply to [].slice.call. Not sure if that will address the issue, but it's an easy fix and worth a shot. Checkout that branch, build, then test your preferred version in /dist. If this fixes your issue I'll publish a patch release. If you see other errors in Sentry, let me know and we'll continue trying to resolve the issues.

jhildenbiddle commented 3 years ago

@bbuhler @tiperes Ping!

bbuhler commented 3 years ago

Hi @jhildenbiddle, I didn't had the chance to reproduce and test your branch yet. I tried using Firefox 24 via Browserstack, but unfortunately the VM browser always became unresponsive and ultimately crashed.

I will give it another try next week.

bbuhler commented 3 years ago

Hi @jhildenbiddle, I was not able to reproduce the issue in a real browser. But as @tiperes mentioned it is reproducible with wkhtmltopdf CLI tool, so I tried with that and it looks like your changes resolves the issue.

Thank you (: