Check all exports for being React components

[gaearon]

Currently exports are not being checked. This is, in a way, a regression from React Hot Loader which used to check every export for componentish-ness. It's hitting us really bad in some cases:

• Hot Reload Actions reduxjs/redux#782
• Changing mapStateToProps doesn't hot reload? reduxjs/react-redux#115
  Explore wrapping HOCs at the call site #18 could potentially fix this, but I have no idea how to even get started on it without a dependency graph, so in the meantime, we should implement exactly what React Hot Loader did: check every export for being a component.

[jamiebuilds]

Oh man, I have no idea how to do this that isn't going to cause a mess of other problems.

[gaearon]

What kind of problems? This is exactly how React Hot Loader currently works, and it's pretty stable.

[jamiebuilds]

You'll have to explain how it works, I would check the tests in the react-hot-loader repo but you don't have any... :for_shame:

[jedwards1211]

@thejameskyle it's just a development tool dude, not for production

[jamiebuilds]

I understand that dude, development tools need tests too.

[gaearon]

Actually that's exactly why this project was started. It was easier to split RHL into several different projects than to write tests for it :-) RHL just enumerated every key in module.exports and wrapped whatever looks like a React component. This is what I suggest to do here: wrap every export into a conservative runtime check. The wrapping function would be like

let wrappedComponents = [];

wrapExport(something) {
  if (
    something &&
    something.prototype &&
    something.prototype.isReactComponent && // only finds Component descendants but good enough
    wrappedComponents.indexOf(something) === -1 // don't wrap twice
  ) {
    wrappedComponent.push(something); // normal generated code would also need to do that 
    return wrapJustLikeWeWrapInGeneratedCode(something);
  }
  return something; // not an export we should hijack
}

Then every export would go through this wrapper, regardless of what's in there.
Does it make sense?

This is a runtime check but it's important for many cases where we want export a React class generated by some factory in another file, and it's essential to maintain its identity.

[solomon-gumball]

+1 Would like to see this.

I am actually working on a backbone/react application and using the react-hmre preset. This is causing react-proxy to run on our backbone views which is an issue!

[gaearon]

Interesting, can you show code that fails? In s separate issue.

[solomon-gumball]

@gaearon will do.

[gaearon]

This is fixed in React Hot Loader 3.
It is built with lessons learned from both React Hot Loader and React Transform.

I’m closing the issue as unsolvable in this repo.