Major change to fix issues with React Hooks

[justin808]

See details in #1198

BREAKING CHANGE

I’m updating React on Rails. The new API for ReactOnRails.register will take an object which registers either

1. React functional or class components
2. render-functions, as defined below.

The update concerns render-functions (formerly called "generator functions"). Previously, generator functions would return JSX and there were sort of like a React function component. But not quite, as you could not use React Hooks within them.

With this update, render-functions need to return either a

1. React Function Component
2. React Class Component
3. An object representing server-side rendering output

Note, you can no longer return JSX!

If you use React component or create a function that returns a React component, then the React on Rails library will call React.createElement and then call ReactDOM.render (or hydrate).

This ensures that React Hooks can always be used with both the React functional or class components and render-functions.

In the case of the render-function, if you could just return the React element (or JSX) and you used the hook API, then you’d get an obscure “hooks don’t work” error.

Use regular React functional or class component or a render-function for your client-side bundle:

// React functional component
export default (props) => (
  <App {...props} />
);

Or a render-function. Note you can't return just the JSX (React element), but you need to return either a React functional or class component.

// React functional component
export default (props, railsContext) => (
  () => <App {{railsContext, ...props}} />
);

Note, this doesn't work, because this function just returns a React Element

// React functional component
export default (props, railsContext) => (
  <App {{railsContext, ...props}} />
);

While this does result in changing all the cases that looked like a React functional component, but had 2 params, I do believe this is worth it.

#1268 Performance?

In my update to React on Rails, I’m changing some APIs. I’m wondering if calling a function to create a function that has a closure with say very a large props JS object, and then returning that function and then
the function will again be called by React.createElement(MyComponent, props)…So the props passed to the React function are not really necessary b/c they can be in a closure.

I’m concerned that having the props passed to the real React component will be bad for performance if they already should be used from the closure. However, JavaScript passes objects by reference, so I don't see any issue.

This could be optimized by not passing any props (which would go into unused param _props below). However, that’s going to be confusing in some cases. I can see people defining the inner component using the same name props and that would shadow the outer one.

import React from 'react';
const MyAppComponent = (props, railsContext) => (
  // NOTE: need to wrap in a function so this is proper React function component that can use
  // hooks
  // the props get passed again, but we ignore since we use a closure
  // or should we
  (_props) =>
      <div>
        <p>props are: {JSON.stringify(props)}</p>
        <p>railsContext is: {JSON.stringify(railsContext)}
        </p>
      </div>
);
export default MyAppComponent;

---

This change is 

[Judahmeek]

Reviewed 21 of 21 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @justin808)

---

CHANGELOG.md, line 28 at r1 (raw file):

### [12.0.0]
#### BREAKING CHANGE
In order to solve the issues regarding React Hooks compatability:

compatibility

---

node_package/src/createReactElement.ts, line 41 at r1 (raw file):

  // TODO: replace any
  let ReactComponent: any;

According to our current types, the return of a RenderFunction should always be a ReactElement.

Seems like we need two types: one for the 2 param generatorFunction & another for the 0-1 param renderFunction, which is what the generatorFunction would return?

---

spec/dummy/client/app/startup/ClientRenderedHtml.jsx, line 20 at r1 (raw file):

// You may do either:
// export default (props, _railsContext) => () => <EchoProps {...props} />;

I'm confused. This is the same as the line above it, which won't work?

[justin808]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Judahmeek and @justin808)

---

spec/dummy/client/app/startup/ClientRenderedHtml.jsx, line 20 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

I'm confused. This is the same as the line above it, which won't work?

One vs two props

[justin808]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Judahmeek and @justin808)

---

node_package/src/createReactElement.ts, line 41 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

According to our current types, the return of a RenderFunction should always be a ReactElement.

Seems like we need two types: one for the 2 param generatorFunction & another for the 0-1 param renderFunction, which is what the generatorFunction would return?

Yes

[justin808]

This makes sense, @Judahmeek. It needs a React function component, not JSX.

[justin808]

An idea on why this changed?

[Judahmeek]

Those are the Ids that you used in your initial commit: be37b34?file-filters%5B%5D=.erb#diff-13675373e6eaa2b6e91995c3ec8974f7

[justin808]

Only pending thing for merge is to double check the CHANGELOG, README, and upgrade docs.

[tahsin352]

There is extra are. Please remove.

[tahsin352]

cool.

[tahsin352]

publish

[justin808]

Thank you @tahsin352 for your review!!!!

@Judahmeek did we get all of these?

[justin808]

Thanks for the help @Judahmeek!

[jacksonrayhamilton]

Glad to see this merged!

Should we expect React on Rails v12 to be released soon?

[simalexan]

@justin808 this is great, appreciate your work!
Just wondering as @jacksonrayhamilton when should we see this published?

[justin808]

OMG! Your thank you messages really fire me up! I'll try to get this one by this weekend!

BTW, if any of you can consider even the tiniest sponsorship, that would be so appreciated!

https://github.com/sponsors/shakacode