Current React lifecycle methods problem

[mtscrb]

I've been trying to "fix a bug" when using react-integration with skatejs components.
I found what is the problem but I'm not sure how to solve it properly. If anyone have any idea / feedback on this I will appreciate it.

You can see the issue here.

So, if you take a look at the console, that button is rendered once without / with missing values and a second time with all the needed properties.

That MAY cause some weird behaviour (in my case, my component flashes when it is instantiated with attributes that changes a default style applied previously).

If you don't use react-integration it won't happen. So digging a bit inside react-integration I found that lifecycle methods in react don't work in sync with skate's or at least that is my assumption.

Trying to make this message not too long, the sequence I see is:

1. Render
   Initial render, that create a react component with only style prop. This is to avoid dynamically computed properties using get and set
   ->
2. componentDidMount
   ->
3. componentWillReceiveProps (called explicitly in componentDidMount) This callback adds all the attributes to the current node, where node is the result of ReactDOM.findDOMNode(this) ( see here ). That will trigger another render in skatejs, but not in the tests (look at the props tests if you are interested) because tests are not using skatejs components only native custom elements.

Naturally I will think of this sequence to be:
componentWillMount (call componentWillReceiveProps)
->
componentWillReceiveProps Here it should set all props in the node
->
render at this point everything should be in place

The problem is, you cannot access findDOMNode in componentWillReceiveProps to add all the properties, so this approach I guess is not possible.

Will be looking forward to any ideas / feedback / correction on any wrong information that I may have written.
Thanks!

[jpnelson]

That will trigger another render in skatejs, but not in the tests (look at the props tests if you are interested) because tests are not using skatejs components only native custom elements.

Why doesn't it rerender natively? I'd expect skate to cause the element to rerender underneath, regardless of whether or not it's a native web component.

[mtscrb]

@jpnelson I guess it's due to skate components lifecycle. But I'm not sure.
While debugging this I realize that it may also be that at the moment its ReactDOM.rendered, the skate component is rendered right away, and after that, componentWillReceiveProps is called causing the second render with all the props.

But I may be wrong, is difficult to tell.

[bradleyayers]

I think we're better to use the ref callback here, in my testing it's called synchronously (or at least before a paint occurs after render()), and would give us the opportunity to call this.componentWillReceiveProps(this.props) and delete the componentDidMount() method.

This means we don't need to play games with setting some props as attributes initially, and then properties later.

I'm thinking something like this:

class ReactComponent extends React.Component {
  static get displayName() {
    return displayName;
  }
  constructor(props) {
    super(props);
    this.onRef = this.onRef.bind(this);
  }
  componentWillReceiveProps(props) {
    const node = ReactDOM.findDOMNode(this);
    this.applyProps(node, props);
  }
  applyProps(node, props) {
    Object.keys(props).forEach(name => {
      if (name === 'children' || name === 'style') {
        return;
      }
      if (name.indexOf('on') === 0 && name[2] === name[2].toUpperCase()) {
        syncEvent(node, name.substring(2), props[name]);
      } else {
        node[name] = props[name];
      }
    });
  }
  onRef(node) {
    if (node != null) {
        this.applyProps(node, this.props);
    }
  }
  render() {
    return React.createElement(tagName, { 
        style: this.props.style,
        ref: this.onRef
    }, this.props.children);
  }
}

[mtscrb]

I really like that approach, and it was my first attempt to fix it. Still, it seems that skatejs render is called before the ref callback, and this doesn't fix the problem.

[treshugart]

If ref is called after connecting the node to the document, render() will always be called before in native, but likely after in polyfill (mutation observers).

[mtscrb]

Using ref callback here + adding observedAttributes in rendered react component, may fix all cases.
There was an ongoing discussion with @bradleyayers (correct me if I'm wrong) about this issue happening not only with observedAttributes not being part of the component at initial render, but it may also happen if you set a property that may change somehow the attributes/dom and are not present at initial render.

Still, I want to make sure that the problem here is not actually how many times render is called initially. It will be nice if it's just 1 time with every props/attributes, but I'm not sure how to do that, or even if it's okey to do that in a "reactified" component.

The main problem is that in those multiple render calls, props/attributes are not the same. This update may tackle both scenarios.

[bradleyayers]

if you set a property that may change somehow the attributes/dom and are not present at initial render.

It's nothing to do with properties changing attributes, as properties (that were declared in props) themselves are the "model" and changes to those will trigger re-rendering.

The main problem is that in those multiple render calls, props/attributes are not the same.

This is a problem, but not a high severity problem. It's only a problem because it requires the component author to gracefully handle the case were only a subset of expected properties/attributes are set.