sheet.addRule API does not work as documented

PavelPZ commented 6 years ago

Hi, first thanks for great JSS tool.

I have a problem with sheet.addRule. Following example for already attached sheet (based on API documentation at https://github.com/cssinjs/jss/blob/master/docs/js-api.md) does not work for me:

const rule = sheet.addRule({ padding: 20})
alert (rule.className)

rule.className is undefined
rule is not added to DOM

When I use sheet.addRule('xxx', { padding: 20}); alert (sheet.classes.xxx), all is working as expected.

I am using JSS ver. 9.0.0 and latest Chrome.

Thank you for clarification.

PavelPZ commented 6 years ago

My testing HTML page looks like:

<html>
<head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/jss/9.0.0/jss.js"></script>
</head>
<body>
  <script>
    var sheet = jss.default.createStyleSheet({}, { index: 1, meta: 'lm-sheet' });
    sheet.attach();
    var rule = sheet.addRule({padding: 20 })
    alert(rule.className)
  </script>
</body>
</html>

Resulting HTML in browser:

<html><head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/jss/9.0.0/jss.js"></script>
  <style type="text/css" data-jss="" data-meta="lm-sheet">
  </style>
</head>
<body>
  <script>
    var sheet = jss.default.createStyleSheet({}, { index: 1, meta: 'lm-sheet' });
    sheet.attach();
    var rule = sheet.addRule({ marginLeft: 20, flex: 1 })
    alert(rule.className)
  </script>
</body></html>

kof commented 6 years ago

className was the option if you want to pass manually a selector, now it is selector option, otherwise selector will be generated, which is accessible through rule.selector

kof commented 6 years ago

Oh I see our documentation is partially outdated.

kof commented 6 years ago

What is your use case for using addRule api? I am thinking how to support your workflow.

kof commented 6 years ago

The problem with the rule.className is that user can set the selector using rule.selector api and then jss needs to figure out className, which is a lot of complexity and actually impossible because selector can contain many class names and more.

kof commented 6 years ago

I need to figure out how to support use cases like yours in a different way.

PavelPZ commented 6 years ago

Thanks a lot, I am JSS beginner

Basically I came to JSS from Fela, which allows to enrich stylesheet on the fly, e.g.

<div className={renderCSS({paddingTop:5})}>Hallo</div>

Here, renderCSS:

generates new CSS rule to DOM
returns class name

I have a lot of react fragments following above pattern. Converting those fragments to HOC (by means of withStyles) is not usefull for me.

kof commented 6 years ago

Try something like this for now, I will think of a better api to support this workflow soon:

const css = (() => {
  let sheet
  let counter = 0

  return (style, options) => {
    if (!sheet) sheet = jss.createStyleSheet().attach()
    const rule = sheet.addRule(`rule${counter++}`, style, options)
    return rule.selector.substr(1)
  }
})()

using it:

const className = css({color: 'red'})

kof commented 6 years ago

Btw. if you are doing this inline <div className={renderCSS({paddingTop:5})}>Hallo</div> you are going to generate the rule again and again on each render. There is no mechanism preventing from this.

kof commented 6 years ago

Ideally you should do something like this:


// generating just once and caching.
const containerClass = css({color: 'red'})

render() {
  return <div className={containerClass}></div>
}

PavelPZ commented 6 years ago

Thanks for workaround and for static containerClass hint. I now understand some JSS-Fela differences (atomic classes in Fela, css rule specificity solution in JSS etc.). I will need to distinguish the static and dynamic parts of my css rules and process them differently (maybe with using Function values).

Will you recomend two sheets for static x function rule values? What is the efficiency of using sheet.update(data) function?

PavelPZ commented 6 years ago

Forgot my last questions please, all is clear. Thanks.

kof commented 6 years ago

sheet.update() is high performance, you can safely do animations with that

PavelPZ commented 6 years ago

I am sorry, but following code does not work, adding rule after attach does not render to sheet:

<html>
<head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/jss/9.0.0/jss.js"></script>
</head>
<body>
  <script>
    var sheet = jss.default.createStyleSheet({}, { index: 1, meta: 'lm-sheet' });
    var rule = sheet.addRule('yyy', { padding: 50 })
    sheet.attach();
    var rule = sheet.addRule('xxx', {padding: 20 })
  </script>
</body>
</html>

Resulting HTML from browser:

<html><head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/jss/9.0.0/jss.js"></script>
<style type="text/css" data-jss="" data-meta="lm-sheet">
.yyy-0-1 {
  padding: 50;
}
</style></head>
<body>
  <script>
    var sheet = jss.default.createStyleSheet({}, { index: 1, meta: 'lm-sheet' });
    var rule = sheet.addRule('yyy', { padding: 50 })
    sheet.attach();
    var rule = sheet.addRule('xxx', {padding: 20 })
  </script>
</body></html>

kof commented 6 years ago

insertRule doesn't display in the textContent, but it does render the rules, if you want to see them, go into dev tools, select the sheet.rules list and see the css rule inside. It will not be reflected in the html view.

PavelPZ commented 6 years ago

Great, thank you @kof

cssinjs / jss

sheet.addRule API does not work as documented #604