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:
// 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)
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.