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

Cannot set property "disabled" of undefined to "true" #144

Closed Azd325 closed 3 years ago

Azd325 commented 3 years ago

This appears in my Sentry without further information. This was not yet reproducible for me in BrowserStack. The browser is Chrome 83

[cssVars(): Cannot set property "disabled" of undefined to "true" , {__cssVars: [Object]}]
jhildenbiddle commented 3 years ago

Interesting, @Azd325.

There is only one place in the code where the ponyfill tries to disable an active stylesheet:

https://github.com/jhildenbiddle/css-vars-ponyfill/blob/9c1b98959205d83777f4cebc10fa89448b0f7690/src/index.js#L516

This occurs after a source stylesheet has been processed and (assuming preserveStatic:true) can be safely disabled.

The fix would be simple:

node.sheet && (node.sheet.disabled = true);

But before I make the change, I'd like to better understand how a <style> or <link> element's sheet property would be undefined. Otherwise, this could be one of several changes needed to prevent similar Sentry errors, and I'd like to avoid releasing multiple patch updates to fix the problem.

If it's possible to reproduce the error in a demo on codesandbox, stackblitz, etc., that would be ideal. Those services don't support IE, but you can set onlyLegacy:false for demo purposes.

Thanks!

Azd325 commented 3 years ago

When and how frequently does this occur? For example, is it only on initial setup or in some way related to your application state (e.g., when a component is being unmounted)?

It is currently very rare we had until now 50 Events occurring with this error. So it currently not easy to track the circumstances what is raising this issue.

Does this happen in multiple browsers?

98% Chrome 83.0.4103 on Windows 7, 2% Firefox 68.0 Windows 10

Is there any way to have Sentry to log additional information about the object? For example, can you identify which stylesheet is causing the error?

Currently, what I read is that Sentry is calling to string on this object and that is the result?!

If it's possible to reproduce the error in a demo on codesandbox, stackblitz, etc., that would be ideal. Those services don't support IE, but you can set onlyLegacy:false for demo purposes.

In IE this error doesn't happen but I could reproduce it.

The only thing was changed since we get this error is that cssVars() is called in a document ready function. Do you think this can be the cause?

jhildenbiddle commented 3 years ago

Hey @Azd325 --

Based on the Sentry error in your original message, it looks like we may be able to identify the node by adding a custom key:value pair. You can try doing this with the ponyfill's onSuccess callback:

cssVars({
  // ... options
  onSuccess: function(cssText, elm, url) {
    elm.__sentryId = elm.outerHTML.slice(0,50);
  }
});

FWIW, this is an odd error. The <link> or <style> node is available, but its sheet is undefined. I can't think of a typical scenario where this would happen. Anything you can think of that may be the cause? Perhaps a CSS-in-JS library or an odd Angular/React/Vue/etc. component state?

If you want to skip all of the investigative work above and just get rid of the error, I have created a branch with the fix mentioned above: https://github.com/jhildenbiddle/css-vars-ponyfill/tree/fix-144. 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 try to address those as well.

Friendly reminder: Be sure to circle back so you don't accidentally end up using the fork indefinitely. :)

Azd325 commented 3 years ago

Thanks I will try.

How you came up with this key elm.__sentryId

jhildenbiddle commented 3 years ago

@Azd325 --

It's just a made up key. I'm assuming it will show up in a Sentry error based on the error you listed in your original comment:

[cssVars(): Cannot set property "disabled" of undefined to "true" , {__cssVars: [Object]}]

Notice the __cssVars: [Object] key:value pair. This is added by the ponyfill to each node that contains a custom property declaration, and it is apparently the only property added directly to the node object (other properties are available via the node's prototype). Therefore, I'm guessing any property we add directly to the node will be displayed in the sentry error. The specifics of the key name or the value are unimportant, so long as we can use them to identify the node that causes the error.

jhildenbiddle commented 3 years ago

Hey @Azd325 --

Checking in to see if you've been able to test the fix branch. I have another fix ready to be released, but if we're only a few days away from confirming this fix for this issue as well than I'll likely wait and bundle them into a single patch release.

Thanks!

jhildenbiddle commented 3 years ago

Fixed in 2.4.2. 🎉