HMR transform breaks React.forwardRef

[haggholm]

Do you want to request a feature or report a bug?

Bug.

What is the current behavior?

I originally filed this as gaearon/react-proxy#82, but the project does not look active (no updates in three years).

In short, if you use the new forwardRef() function on a component derived from (say) React.Component, the result is not a component object but

{
  $$typeof: Symbol(react.forward_ref),
  render: forwardFn(props, ref) { ... }
}

This, then, causes an exception in react-proxy, which expects a prototype; react-proxy is a dependency of metro via react-transform-hmr.

Curiously, it does not happen when the component passed to forwardRef derives an intermediate base class (itself derived from React.Component), perhaps because it’s an ES6 class at transpile time?

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

I do not currently have a minimal example. If needed, I can try to create one; I cannot share the code we have as it’s proprietary.

What is the expected behavior?

I would expect forwardRef to work regardless of whether I’m using the metro Babel preset or not.

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

Node v10.4.0
npm v6.1.0
metro v0.45.6
OS: Ubuntu 18.04

[alex-mcleod]

I've also been experiencing this issue and after doing some digging I've figured out what is going on.

In metro/src/Server.js.flow the hot option is enabled in _getOptionsFromUrl, and currently there is no way to prevent this from happening even if you are not using HMR.

This option is later checked in metro/src/reactNativeTransformer.js.flow and because it is is true, the react-transform-hmr transform gets enabled:

if (options.dev && options.hot) {
    const hmrConfig = makeHMRConfig(options, filename);
    config = Object.assign({}, config, hmrConfig);
}

As you mentioned @haggholm, react-transform-hmr relies on react-proxy which has not been updated to handle components wrapped in React.forwardRef. The reason this issue does not appear if you are using a custom base class for components is that react-transform-hmr is a transform which is applied by metro-babel7-plugin-react-transform (or babel-plugin-react-transform on older versions of React Native), which only looks for these specific super classes to apply transforms to: ['React.Component', 'React.PureComponent', 'Component', 'PureComponent'].

I'm not sure what the best fix is for this. A long term solution would likely be switching to the new React Hot Loader (https://github.com/gaearon/react-hot-loader) in Metro. Another solution would be to allow the hot option to be disabled when starting Metro. As a quick short term fix, you can run sed -i '' 's/hot:[[:space:]]true/hot: false/g' node_modules/react-native/node_modules/metro/src/Server.js in a post install script to disable the hot option. This will break hot module reloading, but otherwise everything seems to work fine.

[haggholm]

Good point. I will likely give that a shot, although I would recommend https://www.npmjs.com/package/patch-package for the job.

[alex-mcleod]

Ah, yes that's a better fix

[rafeca]

Hey, thanks for reporting and apologies for the delay answering.

To give some context, some time ago we decided to enable the HMR transform always for development: this allows Metro to reuse the whole transformed bundle when switching HMR from the device, to not have to wait until the whole bundle gets retransformed (which can be quite slow for huge bundles).

While we could revert that behaviour and make Metro only enable the HMR transform when HMR is on, this won't fix the root issue.

Regarding a more long term solution, I'm supportive to start using react-hot-loader in Metro. If anybody wants to do such migration I'll happily provide help/code pointers 😃 (I'm not familiar with the limitations/tradeoffs compared to the current solution, so it would be good to do some initial investigation).

/cc @gaearon

[gaearon]

As you mentioned @haggholm, react-transform-hmr relies on react-proxy which has not been updated to handle components wrapped in React.forwardRef.

Why can't we update it to support them? I'd be happy to release a fix if you send one.

Regarding a more long term solution, I'm supportive to start using react-hot-loader in Metro.

It has the same issues as the existing solutions. Let's not do this. We have a better long term solution in mind.

[alex-mcleod]

Thanks for your feedback @gaearon and @rafeca, I will have a go at updating react-transform-hmr and react-proxy.

[mmerickel]

Using react-native 0.57.8, metro 0.48.5, react-redux v6 and redux-form v8 this issue has popped up for me. I'm using patch-package to fix it right now via:

patch-package
--- a/node_modules/metro/src/Server.js
+++ b/node_modules/metro/src/Server.js
@@ -1067,7 +1067,7 @@ class Server {
       dev,
       minify,
       excludeSource,
-      hot: true,
+      hot: false,
       runModule: this._getBoolOptionFromQuery(urlObj.query, "runModule", true),
       inlineSourceMap: includeSource,
       platform,

[DimitryDushkin]

Another solution without patching — #313 (comment)

[gaearon]

Again, if there’s a bug or missing thing in react-proxy I’d be happy to take a fix and close this. Did anyone send a PR?

[gaearon]

We've replaced the HMR transform with a different one that doesn't suffer from this issues.
The fix is expected to land in React Native 0.61.