cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.06k stars 397 forks source link

"Rule is not linked" error for function rules with non-canonical selector formatting #815

Closed felthy closed 5 years ago

felthy commented 5 years ago

Minimal reproduction: https://codesandbox.io/s/016vzn15ol

If I define two dynamic rules like this:

  nospace: {
    "@media (min-width:1px)": {
      color: ({ color }) => color
    }
  },
  space: {
    "@media (min-width: 1px)": {
      color: ({ color }) => color
    }
  },

The first one does not work, but the second one does. The difference is the missing space between min-width: and 1px.

I’ve tracked this down to RuleList.link() where the key given by DomRenderer.getKey() is sanitised by the browser, whereas the key originally used in this.map was not.

I’m not sure the best way to fix this. Now that I know the problem, the workaround is obvious, but it seems like something we should fix because it was quite difficult to figure out why my rules weren’t working.

This is the same problem first reported by @jpetitcolas here, in whose case the error was caused by using :after instead of the more correct ::after.

HenriBeck commented 5 years ago

We could write a plugin, or a linter that checks for those things in development, maintaining all of these things which are being sanitized by browsers isn't possible.

The linter would be naturally more complicated though.

kof commented 5 years ago

I was thinking about this also while looking at the complex escaping logic https://github.com/cssinjs/jss/blob/3ccac24bc31837d2727f4b51838f2b629e46871d/packages/jss/src/renderers/DomRenderer.js#L137

I think we should get rid of linking altogether by always using insertRule api when its a function rule/value. So that we don't have to do this fragile linking logic at all.