cssinjs / styled-jss

Styled Components on top of JSS.
http://cssinjs.org/styled-jss
MIT License
217 stars 25 forks source link

Higher order component name #50

Closed goodmind closed 6 years ago

goodmind commented 6 years ago

If component have displayName like this hoc(Component) JSS can't insert rule into css

Warning: [JSS] Can not insert an unsupported rule 

.withStyles(Typography)-6-0-1 {
  flex: 1;
}
lttb commented 6 years ago

Hi! Good point, thanks!

It looks like we should format component's name to the valid css classname. And I think that we can use it only in dev env, but in prod this name may be just a some short key

goodmind commented 6 years ago

@lttb it looks like material-ui can't be used with styled-jss properly because theirs special classes prop

lttb commented 6 years ago

@goodmind I'm not sure that I understood you right, but there are no conflicts with classes prop, styled-jss will just compose components' className from props with it's own, that will be passed to the styled component

here is an example w/ styled-jss and material-ui: https://www.webpackbin.com/bins/-KwmnwyVUgd9R6wiQanz

please note that I've used displayName there as a workaround for this issue, but I'll try to resolve it soon

goodmind commented 6 years ago

@lttb I found out that you can just escape ( and ) in css https://mathiasbynens.be/notes/css-escapes

lttb commented 6 years ago

@goodmind great, thanks! yeah, it looks like it would be enough to escape ( and ) considering real-life possible displayNames

goodmind commented 6 years ago

But it should be escaped only when injected to css, not in html. It's easier to just replace ( and ) with supported characters I think

goodmind commented 6 years ago

So this basically something like this

const escapeParens = (ruleName: string) => ruleName
  // $FlowIgnore
  .replace('(', String.raw`\(`)
  // $FlowIgnore
  .replace(')', String.raw`\)`)

jss.use({
  onProcessRule: (rule) => {
    if (rule.type === 'style' && rule.key.includes('(') && rule.key.includes(')')) {
      rule.selectorText = escapeParens(rule.selectorText)
    }
  }
})

Don't know if this should be separate JSS plugin or can be used as is here

lttb commented 6 years ago

@goodmind I think it's an interesting approach, but on the other hand this plugin with escaping will be called on each rule processing (for example on function values updates too), and this is not the best thing for the core performance.

so I decided to escape the component's name once on the styled-component creation, and that name will be reused for the all other calculations too

kof commented 6 years ago

wait, it is fixed in jss already!

lttb commented 6 years ago

oh, sure, it will be fixed by update to v9.3.1 too