React.render reloads iframes when rendering with a new component

[Gozala]

I've run into this issue when integrating live reloading into our toolchain. Example is obviously convoluted as it tries to capture our setup and also illustrate the issue. The problem is once we swap modules we end up with a new sets of react component definitions and even though their keys and render output is equal iframe still seem to get reloaded.

var Thunk = function(props, context) {
  React.Component.call(this, props, context);
}
Thunk.for = function(view, key) {
  var ReactComponent = function(props, context) {
    Thunk.call(this, props, context)
  }
  ReactComponent.prototype = Object.create(Thunk.prototype);
  ReactComponent.displayName = key.split('@')[0];
  return ReactComponent
}
Thunk.prototype = Object.create(React.Component.prototype);
Thunk.prototype.shouldComponentUpdate = function(props) {
  this.props.model !== props.model
}
Thunk.prototype.render = function() {
  return this.props.view(this.props.model)  
}

let render = (key, view, model) => {
  let  component = view.reactComponent ||
                   (view.reactComponent = Thunk.for(view, key));
  return React.createElement(component, {key, view, model});
}


let redFrame = ({src}) => React.DOM.iframe({
  key: 'frame',
  style: {border: '1px solid red'},
  src
})

let blueFrame = ({src}) => React.DOM.iframe({
  key: 'frame',
  style: {border: '1px solid blue'},
  src
})

React.render(render('main', redFrame, {src: 'https://facebook.github.io/react/docs/getting-started.html'}), document.body)

setTimeout(function() {
  React.render(render('main', blueFrame, {src: 'https://facebook.github.io/react/docs/getting-started.html'}), document.body)
}, 3000);

P.S. I also tried to keep the same root component to keep the identical data-reactid attributes across React.render calls but iframes still reload.

Edit: Updated example so view from props is used and note stored as property of the class which seemed to cause confusion

[Gozala]

Also posting example that illustrates issue for cases where same data-reactid keys are used:

var Thunk = function(props, context) {
  React.Component.call(this, props, context);
}
Thunk.for = function(view, key) {
  var ReactComponent = function(props, context) {
    Thunk.call(this, props, context)
  }
  ReactComponent.prototype = Object.create(Thunk.prototype);
  ReactComponent.displayName = key.split('@')[0];
  return ReactComponent
}
Thunk.prototype = Object.create(React.Component.prototype);
Thunk.prototype.shouldComponentUpdate = function(props) {
  return this.props.view !== props.view ||
         this.props.model !== props.model
}
Thunk.prototype.render = function() {
  return this.props.view(this.props.model)  
}

let render = (key, view, model) => {
  let  component = view.reactComponent ||
                   (view.reactComponent = Thunk.for(view, key));
  return React.createElement(component, {key, view, model});
}


let redFrame = ({src}) => React.DOM.iframe({
  key: 'frame',
  style: {border: '1px solid red'},
  src
})

let blueFrame = ({src}) => React.DOM.iframe({
  key: 'frame',
  style: {border: '1px solid blue'},
  src
})

let app = (model) => React.DOM.div(null, [
  React.DOM.div(null, React.DOM.input({value: model.src})),
  render('frame', frame, model)
])

frame = redFrame

React.render(render('main', app, {src: 'https://facebook.github.io/react/docs/getting-started.html'}), document.body)

setTimeout(function() {
  frame = blueFrame
  React.render(render('main', app, {src: 'https://facebook.github.io/react/docs/getting-started.html'}), document.body)
}, 3000)

Edit: Updated example so view from props is used and note stored as property of the class which seemed to cause confusion

[sophiebits]

Two things determine whether a component is reused: the type and the key. If either changes, the component is unmounted and remounted. See:

https://github.com/facebook/react/blob/3dbdb63a7dbdf31522a623bd627a64c230cdfdcf/src/renderers/shared/reconciler/shouldUpdateReactComponent.js

Since your view.reactComponent value is different in the two cases, the component gets remounted.

[Gozala]

Two things determine whether a component is reused: the type and the key. If either changes, the component is unmounted and remounted. See:

https://github.com/facebook/react/blob/3dbdb63a7dbdf31522a623bd627a64c230cdfdcf/src/renderers/shared/reconciler/shouldUpdateReactComponent.js

@spicyj thanks for the pointer and explanation that was my guess of what was going on as well.

Since your view.reactComponent value is different in the two cases, the component gets remounted.

@spicyj my question is if there is any way I could make it less destructive ? In fact I would like node to be unmounted in sense that I'd like it to clean up listeners etc... But ideally node itself would remain in the tree.

Also primary reason we generate component per view function vs reusing a same Thunk component is to have a displayName which are used by all react devtools. If that was property or better yet a method on the component we could reused same Thunk component.

@spicyj so if HTML was already pre-rendered on the server would react still throw away pre-existing nodes or would it just patch them ? If later is there any way I could trigger that code path and there for avoid remounting ?

[sophiebits]

No, there's no way (currently) to separate the unmounting process from the actual DOM manipulation. From your code it looks like you can have a different Thunk for each key and cache the ones you've created so far. Would that work?

Adopting server rendering is a special case for the initial render and there's no way to get that behavior on an update.

[Gozala]

Posting some relevant discussion from IRC with @petehunt

•petehunt> gozala: yea will do, what i would do is just have a mapping of key->reactComponent and fill it when there’s a miss
11:55 AM <•petehunt> but will look a bit later, sorry!
11:55 AM → enaqx joined (~enaqx@188.163.69.11)
11:55 AM <•petehunt> basically memoize Thunk.for() and pass view via props

So in Thunk.for creates component and caches it by storing it on the passed view function, that way same view function will end up with a same Thunk component. I also updated examples to use props.view as I believe original code caused confusion.

The reason only reason same Thunk can not be reused for all the views is because that way we would end up with a same displayName for all the views in the app and that's not ideal. We could probably have map of displayName -> Thunk to workaround but we will end up growing that map and never clearing entries which is not ideal. Although maybe we could use componentWillMount and componentWillUnmount hooks to increment decrement number of users and remove entry when it reaches 0 ?

Still the primary issue is though that all of the JS code is reloaded so definition of Thunk would also be new unless map is stored somewhere globally. I might be able to workaround that too, but if there is some way to cause react just render over vs mounting that would be the best option.

[Gozala]

I managed to resolve this issue by following suggestions made, posting below update of the original snippet for the reference:

// Following symbol is used for cacheing Thunks by an associated displayName
// under `React.Component[thunks]` namespace. This way we workaround reacts
// remounting behavior if element type does not match (see facebook/react#4826).
var thunks = (typeof(Symbol) !== 'undefined' && Symbol.for) ?
    Symbol.for('reflex/thunk/0.1') :
    `@@reflex/thunk/0.1`;

// Alias cache table locally or create new table under designated namespace
// and then alias it.
var thunksByDisplayName = React.Component[thunks] ||
                          (React.Component[thunks] = Object.create(null));


var Thunk = function(props, context) {
  React.Component.call(this, props, context);
}
Thunk.withDisplayName = function(displayName) {
  const NamedThunk = function(props, context) {
    Thunk.call(this, props, context);
  }
  NamedThunk.displayName = displayName
  NamedThunk.mounts = 0
  NamedThunk.prototype = Object.create(Thunk.prototype, {
    constructor: {value: NamedThunk}
  });
  NamedThunk.prototype.render = Thunk.prototype.render
  return NamedThunk
}
Thunk.prototype = Object.create(React.Component.prototype, {
  constructor: {value: Thunk}
});
Thunk.prototype.componentWillMount = function() {
  // Increase number of mounts for this Thunk type.
  ++this.constructor.mounts
}
Thunk.prototype.componentWillUnmount = function() {
  // Decrement number of mounts for this Thunk type if no more mounts left
  // remove it from the cache map.
  if (--this.constructor.mounts === 0) {
    delete thunksByDisplayName[this.constructor.displayName];
  }
};
Thunk.prototype.shouldComponentUpdate = function(props) {
  return this.props.view !== props.view ||
         this.props.model !== props.model
}
Thunk.prototype.render = function() {
  return this.props.view(this.props.model)  
}

let render = (key, view, model) => {
  const name = key.split('@')[0];
  const NamedThunk = thunksByDisplayName[name] ||
                     (thunksByDisplayName[name] = Thunk.withDisplayName(name));

  return React.createElement(NamedThunk, {key, view, model});
}


let redFrame = ({src}) => React.DOM.iframe({
  key: 'frame',
  style: {border: '1px solid red'},
  src
})

let blueFrame = ({src}) => React.DOM.iframe({
  key: 'frame',
  style: {border: '1px solid blue'},
  src
})

React.render(render('main', redFrame, {src: 'https://facebook.github.io/react/docs/getting-started.html'}), document.body)

setTimeout(function() {
  React.render(render('main', blueFrame, {src: 'https://facebook.github.io/react/docs/getting-started.html'}), document.body)
}, 3000);

[Gozala]

That being said this did not fully covered my use case as while our primarily is using render (similar to one in the example) there are still handful of plain react components to workaround few other limitations. For that I basically end up wrapping React.createElement and React.findDOMNode in order to always use same Proxy component to which I pass actual component type as prop. I'm little worried about the robustness of this solution so I'd welcome any comments if @spicyj or @petehunt will have few moments to spare to look at it:

Gozala/browser.html@731e48e