Styled-jsx: CSSOM throws an error when a rule is not supported

Created on 7 Oct 2017  Â·  10Comments  Â·  Source: vercel/styled-jsx

When a rule is not supported by the browser sheet.insertRule throws an error (and screws up the stylesheet it seems). This is unfortunate because we automatically enable optimizeForSpeed only in PROD so users may realize or find out about the bug too late.

eg.

Uncaught DOMException: 
Failed to execute 'insertRule' on 'CSSStyleSheet': 
Failed to parse the rule 'input::-ms-clear { display: none } '.

EDIT:

I dug a bit more and the bug is somewhere else. When insertRule fails we still increase sheet length i.e. the rule index but obviously no rule is inserted. As result of this things start to breaks since indices are not correct anymore.


vvvvv Ignore the part below vvvvv

Leaving the description just for completeness

We can "fix" this in two ways:

  1. Throw a proper message explaining what is going on and suggest to do feature detection.
  2. We add some sort of check - might be expensive and affect perf though:
// note this is an idea and needs validation but it might work
const testStyle = document.createElement('style')
document.head.appendChild(testStyle)
const testSheet = testStyle.sheet
document.head.removeChild(testStyle)

function isValidRule(rule) {
  try {
    testSheet.insertRule(rule)
    return true
  } catch (err) {
    return false
  }
}

// ...

isValidRule(rule) && realSheet/*no phun intended*/.insertRule(rule)
bug help wanted

All 10 comments

cc @rauchg @nkzawa @arunoda

Throw a proper message explaining what is going on and suggest to do feature detection.
We add some sort of check - might be expensive and affect perf though:

Sounds good to me if we do this only on development.

Additionally, we'd need a way to not apply optimizeForSpeed for specific styles, so that you can still use input::-ms-clear while optimizing others ?

Does it mess up the whole stylesheet or just everything after the failed insert?

Edit: It seems to just affect the current insert.

var sheet = document.head.appendChild(document.createElement('style')).sheet
var length = sheet.cssRules.length

try {
    // passes
    sheet.insertRule('body{background:red;}')
    // fails
    sheet.insertRule('input::-ms-clear { display: none }', length++)
} catch (e) {
    // retry works
    sheet.insertRule('body{background:blue;}', length)
}

@thysultan yep that might be the case i.e. try/catch prevents the failure. We have it https://github.com/zeit/styled-jsx/blob/master/src/lib/stylesheet.js#L121 but now I realized that we use it in another place without safeguard https://github.com/zeit/styled-jsx/blob/master/src/lib/stylesheet.js#L145

Need to test that when I am back home

I think I had this issue yesterday when I updated to next 4.0.0-beta.3 from beta.2. I haven't had time to look into the specifics yet, but it was quite confusing to diagnose as everything seemed fine in dev but after refactoring a fair few components and testing on prod, the issue presented itself quite acutely. If it failed, it seemed to break certain scoped styles on a subsequent page. The only error message I encountered was Failed to execute 'deleteRule' on 'CSSStyleSheet'.

Yep it works, I dug a bit more and the bug is somewhere else. When insertRule fails we still increase length i.e. the rule index and therefore things started to break.

I don't think it has to do with the rule being invalid css, but that insertRule doesn't support adding it in that way. i.e valid variants @import, @viewport, @counter-style, @document rules also throw.

@thysultan @import is fine https://jsfiddle.net/gtdj6725
The issue was with my code not handling failures properly, specifically indices were updated in the wrong way https://github.com/zeit/styled-jsx/pull/296/files Let me know in case you think that this might still fail somehow – I think that it explains the missing styles issue we were experiencing.

Yes, that seems to work, i guess it's only @viewport {width: device-width;}, @counter-style list {color:red;} and @document {body{color:red;}} that are valid that don't seem to work with insertRule.

cool good to know! We could fallback to regular style tags when we detect those rules.
I will do it in a follow up, I don't think it is urgent.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

timsuchanek picture timsuchanek  Â·  29Comments

thisbejim picture thisbejim  Â·  18Comments

rauchg picture rauchg  Â·  18Comments

rauchg picture rauchg  Â·  25Comments

jdolinski1 picture jdolinski1  Â·  16Comments