Style updates silently fail with trailing semicolon in style

[brigand]

On the first render it works, but then the update silently fails. Expected would be the semicolon being removed, or an error/warning in the development build.

Tested on Chrome latest, Firefox latest, with 0.12.1 and master.

Minimal: jsbin

var App = React.createClass({
  getInitialState: function(){
    return {color: 'red'}
  },

  handleClick : function() {
    console.log('hello world');
    this.setState({
      color: 'blue'
    });
  },

  render: function() {
    console.log('rendering');
    var style = {
      backgroundColor: this.state.color
    };

    if (this.props.withsemi) {
      style.backgroundColor += ";";
    }

    return (
    <div>
      <div style={style} onClick={this.handleClick}></div>
    </div>
    )
  }
});

@rabishah ran into (on IRC).

[zpao]

My first instinct is to put a warning in DEV so we don't incur any costs in prod. This would be similar to our other style key warning ("you used background-color but probably meant backgroundColor" or whatever it is we say).

warning(
  !/;$/.test(value),
  'yo, no semicolons, it breaks the things'
);

[brigand]

@zpao, sounds reasonable, I don't think there's any valid reason to have a semicolon there.

[syranide]

@zpao It's only a warning, but it's worth considering that url(foo;bar) is valid CSS IIRC.

[zpao]

/;$/.test('url(foo;bar)') === false so that wouldn't warn :)

[syranide]

@zpao Oh right $ :), but I've seen plenty of invalid background: '#000; background: #333', but perhaps that's an exercise for some other time. Anyway, you might want to ignore white-space after the ; as well.

[mzabriskie]

I'm working on this issue. I've added a warning, but thinking that it may be a good idea to trim any semicolons as well. The warning will serve to let developers know it's invalid syntax, but try and fix the problem while we're at it.