add render component extension point

[EdmondChuiHW]

Summary

Add extension point to delegate component rendering to an override-able function, similar to getConstructor. Also add callback to component unmount

This allows custom implementations that renders parent wrapper components, e.g.

function renderWithContext(renderFunctionName, component, node, props) {
  const contextObj = JSON.parse(node.getAttribute("data-context-object"));

  ReactDOM.render(
    <SomeReactContext.Provider value={contextObj}>
      {component}
    </SomeReactContext.Provider>,
    node,
  )
}

Hot reloading

Built-in support for hot reloading 🔥 would also be added via this extension point. See this PR: #1065

Stack-ability

It is also (optionally) designed to be "stackable". Allowing conditional overrides based on env var or any arbitrary boolean:

const baseRender = ReactRailsUJS.renderComponent;
ReactRailsUJS.renderComponent =(renderFunctionName, component, node, props) => {
  const contextObj = JSON.parse(node.getAttribute("data-context-object"));

  baseRender(
    renderFunctionName,
    <SomeReactContext.Provider value={contextObj}>
      {component}
    </SomeReactContext.Provider>,
    node,
    props,
  )
}

in another file:

if (!isDev()) return;

const baseRender = ReactRailsUJS.renderComponent;
ReactRailsUJS.renderComponent =(renderFunctionName, component, node, props) => {
  const hotReloadedComponent = /* == hot reload logic == */;

  baseRender(
    renderFunctionName,
    hotReloadedComponent,
    node,
    props,
  )
}

In the above example, the final render "stack" should look like:

// in dev:
HotReloadWrappers
  ContextProviders
    OriginalComponent

// in prod:
ContextProviders
  OriginalComponent

Other Information

Inline comments have been added to discusses options considered and their tradeoffs.

Also uncertain on how to fix the Travis CI build error

[EdmondChuiHW]

folder structure and import pattern is taking reference from getConstructor for consistency

[EdmondChuiHW]

The comment and pattern here intends to mirror getConstructor. Let me know if

1. the function signature can be better represented
2. if the type of renderFunctionName ("hydrate" | "render") should be made more generic, i.e. string
3. export the renderWithReactDOM function or assign it in a constant, e.g. ReactRailsUJS.DEFAULT_RENDER_COMPONENT_FUNCTION

Reference:

react-rails/react_ujs/index.js

Lines 63 to 66 in d5da111

	// Get the constructor for a className (returns a React class)
	// Override this function to lookup classes in a custom way,
	// the default is ReactRailsUJS.ComponentGlobal
	getConstructor: constructorFromGlobal,

[EdmondChuiHW]

Assignment to component is removed here. It appears unused, since the function returns immediately afterwards. In addition, the return value from ReactDOM.render has been marked as deprecated.

ReactDOM.render() currently returns a reference to the root ReactComponent instance. However, using this return value is legacy and should be avoided because future versions of React may render components asynchronously in some cases. If you need a reference to the root ReactComponent instance, the preferred solution is to attach a callback ref to the root element.

[EdmondChuiHW]

The last argument props here is unused in this default render function. However, it is provided to support extensibility for custom implementations. The hot reload variation, for uses props to re-render with a freshly-reloaded component constructor.

Let me know if

1. function should declare the props argument even if it's unused (for self-documentation/ease of copy-and-pasting)
2. function signature should change to pass constructor instead of/in addition to component. The existing implementation of cached turbolinks components would need extra considerations if component is not passed to this function

Previous proposal referenced: #595 (comment)

• from

ReactDOM.render(React.createElement(constructor, props), node);

• to

ReactRailsUJS.renderComponent(node, constructor, props)

// .... then ...
renderComponent: function(node, constructor, props) {
  ReactDOM.render(React.createElement(constructor, props), node);
},

[EdmondChuiHW]

Why props is required where component is already passed in? Hot reload requires a "fresh" constructor and component while re-rendering. props is required to be passed onto the "fresh" constructor. #1065 (comment)

[EdmondChuiHW]

Considerations for removing props from the render function: while it is trivial to obtain props via node#getAttribute and JSON.prase inside this render function, the work is already done in mountComponents.

Furthermore, existing implementation of turbolinks would likely be required to move here in such case; this increases the complexity to override the render function. The intention is to keep the render function as simple as possible such that it has minimal impact to existing features when it's overriden

I do not have benchmarks/metrics to support a performance argument

[BookOfGreg]

This is looking good, I'll need some time to check compatibility of this with existing codebases and I may look at adding tests.
All tests in Travis pass except Webpack 4, could be a random bug specific to that set of builds.
The accompanying docs/comments read well and thank you for that.

[souporserious]

Any status on this? Would love to have something more first-class like this for wrapping top-level components!

[justin808]

@EdmondChuiHW, @souporserious given the discussion on #982, do you agree that this PR is no longer needed?

If you think it's still useful, please request to reopen this.