Fixed issue for dynamic props update

[wellguimaraes]

{
  color: props => props.active ? 'red' : 'black',

  // it was crashing by having this conditional rule here
  '@media screen': {
    background: props => props.active ? 'white' : 'gray'
  }
}

[coveralls]

Coverage remained the same at 100.0% when pulling fb8cd97 on wellguimaraes:master into d3a578a on cssinjs:master.

[lttb]

@wellguimaraes, thank you for this fix, that's great!

[lttb]

The problem here is that in this case we update the whole StyleSheet, and it will reduce performance significantly. That's why we update by the name, otherwise we can replace all that code with just this.sheet.update :).
Another problem with such update is that we will rerender rules for other instances, that were not really updated.

But this solution should work:

if (rule.name) this.sheet.update(rule.name, props)
if (rule.rules) rule.rules.update(props)

Conditional rules has their own RulesContainer in rules, so with .update we should iterate only for needed rules.

[lttb]

@kof what do you think?

[lttb]

There are also some issues with codestyle.
That's strange that pre-commit linting dit not work 🤔

Anyway I'll setup linting by CI too

[coveralls]

Coverage remained the same at 100.0% when pulling 12601ea on wellguimaraes:master into f35db97 on cssinjs:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 8bc484b on wellguimaraes:master into f35db97 on cssinjs:master.

[lttb]

we should use assertSheet here. but I checked that getCss returns empty media query rule for function values like:

.button-1-id{padding:10px;}@mediascreen{.button-1-id{}}

for static values it's ok.

need to understand why dynamic properties didn't render with values for conditional rules

[lttb]

I think it would be better to name this test like 'should update dynamic props for conditional rules'

[lttb]

@wellguimaraes please rebase your branch by origin, I've fixed Travis config and eslint checks

[lttb]

let's save sheet in variable

[lttb]

@wellguimaraes It looks like it still does not work for me (with whole sheet update too)

https://codesandbox.io/s/lYL3Jz3l7

need to understand issues with rendering of dynamic conditional rules

[lttb]

There is an issue w/ jss: cssinjs/jss#511

[coveralls]

Coverage remained the same at 100.0% when pulling d03282c on wellguimaraes:master into 084d92a on cssinjs:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 829864b on wellguimaraes:master into 084d92a on cssinjs:master.

[lttb]

@wellguimaraes it looks like we have fix it, is it okay now?

it works here: https://codesandbox.io/s/M8o5p00rP

[wellguimaraes]

awesome!