Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
felthy opened this issue Aug 6, 2018 · 2 comments · Fixed by #857
Closed
Labels
bug It went crazy and killed everyone.

Comments

@felthy
Copy link
Member

felthy commented Aug 6, 2018

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.

@felthy felthy added the bug It went crazy and killed everyone. label Aug 6, 2018
@HenriBeck
Copy link
Member

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
Copy link
Member

kof commented Aug 13, 2018

I was thinking about this also while looking at the complex escaping logic

const getUnescapedKeysMap = (() => {

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It went crazy and killed everyone.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants