Move instrumentation from isomorphic to renderers/shared

[gaearon]

Per @sebmarkbage, we want isomorphic folder to only contain the code necessary to author React components. It should not have dependencies on ReactInstrumentation, ReactDebugTool, and devtools/* friends, which themselves should be moved to renderers/shared/.

The only caller we to ReactInstrumentation we have right now in that folder is onSetState() inside ReactComponent. We need to move it somewhere inside the renderer (e.g. in the update queue implementation).

Labeling it as a good first bug because this mostly involves moving files. The goal is that files in isomorphic folder don’t contain anything that references ReactInstrumentation.

This is up for grabs but I’ll do it myself if no one takes it.

[iamdustan]

Should ReactPerf also move out to renderers/shared/?

[sebmarkbage]

Yea.

@gaearon Do you have a plan for how the new perf tool API will be used since it is tied to a particular renderer? It is the only of the add-ons that does that.

[gaearon]

I don't quite understand what you're asking. From user's point of view it is used the same way as before, through react-addons-perf. If you create a third party renderer you'll need to emit some events to tell it what's going on, but I don't see this as being "tied" to RN or DOM renderers.

[sebmarkbage]

@gaearon react-addons-perf currently requires react/lib/ReactDefaultPerf. After #6795 there won't be such a file because it will live in react-dom/lib/ReactDefaultPerf. However, that only works for DOM. For React Native, we'd have to require react-native/lib/ReactDefaultPerf.

[gaearon]

I still don’t quite understand what you want to do in #6795. Why is react-addons-perf affected by this but other addons are not?

[sebmarkbage]

Currently both react-dom and react-native uses the same instance of all the shared modules. It uses runtime dependency injection to determine which environment actually gets used. The idea is to copy all the modules to each one of them, except the isomorphic ones. That way multiple renderers can coexist on npm, but more importantly they can be versioned independently.

This means that there will be two copies of the ReactPerf modules on npm. One in each renderer.

The alternative would be that we keep ReactPerf and the debug tool in isomorphic and have renderers register themselves there somehow.

The other addons are not affected because they only depend on things in the isomorphic folder which will remain shared.

[gaearon]

Do I understand correctly that you are concerned that changes to ReactDebugTool (e.g. adding new events) will force us to release isomorphic react more often, and will force us to keep react-dom and react-native versioned more closely together than we would have preferred otherwise?

[iamdustan]

Is the intent also to get away from the current dependency injection? Would this allow multiple independent renderers to coexist in a runtime environment as well? (Possible upcoming scenario: ReactDOM and ReactART, with ReactDOM using the current reconciler and ART using the incremental reconciler)

[sebmarkbage]

@iamdustan Yes, that is an imminent upcoming scenario.

[sebmarkbage]

@gaearon I'm more concerned about the events not making sense across renderers, but I guess we already tried to make that very generic so maybe that's unfounded.

I'm concerned about having too much in the isomorphic package. It is supposed to be small.

I'm also concerned about having stateful modules in the isomorphic package. It would be nice to let react be a direct dependency instead of peerDependency. But there are also practical issues such as that it is difficult that beginX and endX calls aren't nested inside each other when there are two different renderers doing different work on the same stateful module.

I think ReactInstrumentation and ReactDebugTool should at least be on a per renderer level.

ReactPerf itself is more like one tool of many that could connect to any number of renderers that have a ReactDebugTool hook. It could live in the react-addons-perf package.

Then we'd need some way to connect them up. For UI based tools such as the Chrome React DevTools, we've used a global variable to coordinate. That way it doesn't matter how npm nests the packages and it doesn't matter which module loads first. The perf tool or the renderer.

Not sure how we'd solve this with a programmatic API though.

One solution would be to expose the ReactInstrumentation module on the renderer and then you have to register it to get a paired up perf tool.

var ReactDOMPerf = require('react-addons-perf')(require('react-dom'));

If we're comfortable with react remaining a peerDependency, we could have every renderer register their instrumentation. react-dom would call:

require('react').registerRendererInstrumentation(require('ReactInstrumentation'));

Then the perf tool can be a summary of all renderers ever registered.

[sebmarkbage]

I'm offering problems more than solutions. Eager to hear your proposals.

Btw, a design goal was to allow third parties doing something like ReactInstrumentation.debugTool = totallyCustomTool; which would have full access to custom profiling etc.

[gaearon]

Thanks, I understand it better now. I created #6812 to track this and will share my thoughts next week.