Move renderer host configs into separate modules

[gaearon]

I tried resolving host configs statically but it's hard right now because the host config is all over the place in the renderer code.

If we want to resolve the host configs statically, we're going to need to put them into separate modules (so that we can use ES imports for better mangling, and to override resolving on the file level).

I'm working just on that step here. It doesn't change anything from the reconciler's point of view. In some cases (such as with test renderer) I'll have to create a third file that keeps mutable private state that's accessible both by the renderer public API and the host config.

I did this for all host configs except react-noop-renderer. This is because we use react-noop-renderer, among other things, for testing the npm react-reconciler package, and the way it's written now with inline reconciler calls reflects how external renderers will continue consuming it. When we add inlining, we'll want to keep that system working.

The diff appears as if there were a lot of changes, but really it's just moving things around and tweaking what imports what. I didn't change the logic.

[pull-bot]

Details of bundled changes.

Comparing: c802d29...53b03e4

react-art

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-art.development.js	0.0%	+0.2%	445.46 KB	445.51 KB	96.61 KB	96.82 KB	UMD_DEV
react-art.production.min.js	0.0%	🔺+0.1%	92.81 KB	92.81 KB	28.24 KB	28.25 KB	UMD_PROD
react-art.development.js	0.0%	0.0%	371.29 KB	371.33 KB	77.93 KB	77.95 KB	NODE_DEV
react-art.production.min.js	0.0%	0.0%	57.31 KB	57.32 KB	17.5 KB	17.5 KB	NODE_PROD
ReactART-dev.js	0.0%	0.0%	380.32 KB	380.36 KB	77.67 KB	77.69 KB	FB_WWW_DEV
ReactART-prod.js	0.0%	🔺+0.1%	186.79 KB	186.85 KB	30.65 KB	30.66 KB	FB_WWW_PROD

react-native-renderer

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
ReactNativeRenderer-dev.js	0.0%	-0.3%	488.66 KB	488.82 KB	104.22 KB	103.93 KB	RN_FB_DEV
ReactNativeRenderer-prod.js	-0.1%	🔺+0.1%	236.9 KB	236.66 KB	39.56 KB	39.61 KB	RN_FB_PROD
ReactNativeRenderer-dev.js	0.0%	-0.3%	488.32 KB	488.48 KB	104.14 KB	103.84 KB	RN_OSS_DEV
ReactNativeRenderer-prod.js	-0.1%	🔺+0.1%	223.12 KB	222.87 KB	37.38 KB	37.43 KB	RN_OSS_PROD
ReactFabric-dev.js	0.0%	-0.0%	471.06 KB	471.11 KB	99.82 KB	99.82 KB	RN_FB_DEV
ReactFabric-prod.js	0.0%	-0.1%	208.74 KB	208.74 KB	34.74 KB	34.72 KB	RN_FB_PROD
ReactFabric-dev.js	0.0%	-0.0%	471.09 KB	471.14 KB	99.84 KB	99.83 KB	RN_OSS_DEV
ReactFabric-prod.js	0.0%	-0.1%	208.77 KB	208.77 KB	34.75 KB	34.73 KB	RN_OSS_PROD

Generated by 🚫 dangerJS

[sophiebits]

It seems like there are three ways we could do this.

1. Split Host Config and Renderer (i.e., this diff)

ReactDOMHostConfig.js:

export default {...};

ReactDOM.js:

import {createContainer, ...} from 'ReactFiberReconciler';
...
export default ReactDOM;

2. Host Config and Renderer as separate exports

I understand that ES6 modules allow circular imports. So I think that means we can do something like:

ReactDOM.js:

import {createContainer, ...} from 'ReactFiberReconciler';
...
export const hostConfig = {...};
export default ReactDOM;

and ReactFiberReconciler can import the host config from here. This way we don't need to split them up, which might be more convenient.

3. Like now, with a little dynamic injection

That is, continue having ReactFiberReconciler export a function. I guess that would need to look something like:

function ReactFiberReconciler(hostConfig) {
  invariant(ReactGlobalHostConfig.current === null);
  ReactGlobalHostConfig.current = hostConfig;
  return require('ReactFiberReconcilerGuts');

And then ReactDOM.js unchanged. Which I'm sure Seb will hate, but it would be nice to do this because then we can match the external API for our internal renderers.

?

I don't feel qualified to know which is best here.

[gaearon]

The approach I wanted to do is similar to what you suggested in (1) except we'd have an export for every method in the config rather than a default export for the whole config.

This is why I want to go all-in on ES6 modules and avoid a reified object config representation. Having config as an object prevents GCC from mangling its property names, and also prevents it from inlining its implementations when they're small enough (such as the case with insertBefore() etc).

In terms of expressing it in code, custom renderers would keep doing

import reconciler from 'react-reconciler';
var Thing = reconciler(config);

but our renderers would do this:

import * as ReactDOM from 'react-reconciler/inline'; // inline = magic!

I can push up a proof of concept for how this could work soon.

---

TLDR:

• (1) is roughly what I'm going for, but I want export for each host config method rather than export default
• (2) looks neat but it either reifies an object host config (do not want) or every export is a config method in which case I find it a bit confusing when combined with things like unmountComponentAtNode which conceptually can also be thought of "exports" even though we don't model them that way now
• (3) reifies an object host config (do not want)

[gaearon]

I also don't think splitting is that inconvenient. In case of RN/RF it got a bit cleaner IMO.

[gaearon]

@bvaughn @acdlite If either of you find the time to review today, I'd be very grateful—this is going to be a pain to keep in sync and I'd like to land before anything else changes in these files :-)

[bvaughn]

I read the discussion on the PR. It makes sense. I also scanned the changes and they seem to be as expected (just splitting things out into the separate host config files as an initial step).

So ... high-level LGTM from me 👍

[gaearon]

I did some build diffing and I think this is correct. DOM bundle looks closest, other bundles have the order of modules switched up a little bit but the end result should be the same.