Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require createRoot/hydrateRoot from 'react-dom/client' under react-dom 18+ #1460

Merged
merged 1 commit into from
Jun 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ Changes since last non-beta release.

To migrate this change, remove `mini_racer` gem from your `Gemfile` and test your app for correct behaviour. You can continue using `mini_racer` and it will be still picked as the default `ExecJS` runtime, if present in your app `Gemfile`.

- Fixed the `You are importing hydrateRoot from "react-dom" [...] You should instead import it from "react-dom/client"` warning under React 18 ([#1441](https://github.com/shakacode/react_on_rails/issues/1441)). [PR 1460](https://github.com/shakacode/react_on_rails/pull/1460) by [alexeyr](https://github.com/alexeyr).

In exchange, you may see a warning like this when building a Webpack bundle under React 16:
```
WARNING in ./node_modules/react-on-rails/node_package/lib/reactHydrateOrRender.js19:25-52
Module not found: Error: Can't resolve 'react-dom/client' in '/home/runner/work/react_on_rails/react_on_rails/spec/dummy/node_modules/react-on-rails/node_package/lib'
@ ./node_modules/react-on-rails/node_package/lib/ReactOnRails.js 34:45-78
@ ./client/app/packs/client-bundle.js 5:0-42 32:0-23 35:0-21 59:0-26
```
It can be safely [suppressed](https://webpack.js.org/configuration/other-options/#ignorewarnings) in your Webpack configuration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahangarha Let's better document this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am concerning about version 16 or 18. I think there is a mistake there. I asked Alexey for clarification.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I mentioned a fix for the phrasing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also put it into the config shakapacker exports. Would that potentially break anything? @justin808


### [13.0.2] - 2022-03-09
#### Fixed
Expand Down Expand Up @@ -1023,7 +1033,8 @@ Best done with Object destructing:
##### Fixed
- Fix several generator related issues.

[Unreleased]: https://github.com/shakacode/react_on_rails/compare/13.0.1...master
[Unreleased]: https://github.com/shakacode/react_on_rails/compare/13.0.2...master
[13.0.2]: https://github.com/shakacode/react_on_rails/compare/13.0.1...13.0.2
[13.0.1]: https://github.com/shakacode/react_on_rails/compare/13.0.0...13.0.1
[13.0.0]: https://github.com/shakacode/react_on_rails/compare/12.6.0...13.0.0
[12.6.0]: https://github.com/shakacode/react_on_rails/compare/12.5.2...12.6.0
Expand Down
34 changes: 22 additions & 12 deletions node_package/src/ReactOnRails.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ReactElement, Component } from 'react';
import type { ReactElement } from 'react';

import * as ClientStartup from './clientStartup';
import handleError from './handleError';
Expand All @@ -13,13 +13,13 @@ import type {
RegisteredComponent,
RenderParams,
RenderResult,
RenderReturnType,
ErrorOptions,
ReactComponentOrRenderFunction,
AuthenticityHeaders,
StoreGenerator
} from './types/index';
import reactHydrate from './reactHydrate';
import reactRender from './reactRender';
StoreGenerator,
} from './types';
import reactHydrateOrRender from './reactHydrateOrRender';

/* eslint-disable @typescript-eslint/no-explicit-any */
type Store = any;
Expand Down Expand Up @@ -172,25 +172,35 @@ ctx.ReactOnRails = {
},

/**
* @example
* ReactOnRails.render("HelloWorldApp", {name: "Stranger"}, 'app');
*
* Does this:
* ReactDOM.render(React.createElement(HelloWorldApp, {name: "Stranger"}),
* document.getElementById('app'))
* ```js
* ReactDOM.render(React.createElement(HelloWorldApp, {name: "Stranger"}),
* document.getElementById('app'))
* ```
* under React 16/17 and
* ```js
* const root = ReactDOMClient.createRoot(document.getElementById('app'))
* root.render(React.createElement(HelloWorldApp, {name: "Stranger"}))
* return root
* ```
* under React 18+.
*
* @param name Name of your registered component
* @param props Props to pass to your component
* @param domNodeId
* @param hydrate Pass truthy to update server rendered html. Default is falsy
* @returns {virtualDomElement} Reference to your component's backing instance
* @returns {Root|ReactComponent|ReactElement} Under React 18+: the created React root
* (see "What is a root?" in https://github.com/reactwg/react-18/discussions/5).
* Under React 16/17: Reference to your component's backing instance or `null` for stateless components.
*/
render(name: string, props: Record<string, string>, domNodeId: string, hydrate: boolean): void | Element | Component {
render(name: string, props: Record<string, string>, domNodeId: string, hydrate: boolean): RenderReturnType {
const componentObj = ComponentRegistry.get(name);
const reactElement = createReactOutput({ componentObj, props, domNodeId });

const render = hydrate ? reactHydrate : reactRender;
// eslint-disable-next-line react/no-render-return-value
return render(document.getElementById(domNodeId) as Element, reactElement as ReactElement);
return reactHydrateOrRender(document.getElementById(domNodeId) as Element, reactElement as ReactElement, hydrate);
},

/**
Expand Down
7 changes: 2 additions & 5 deletions node_package/src/clientStartup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import type {

import createReactOutput from './createReactOutput';
import {isServerRenderHash} from './isServerRenderResult';
import reactHydrate from './reactHydrate';
import reactRender from './reactRender';
import reactHydrateOrRender from './reactHydrateOrRender';

declare global {
interface Window {
Expand Down Expand Up @@ -168,10 +167,8 @@ function render(el: Element, railsContext: RailsContext): void {
throw new Error(`\
You returned a server side type of react-router error: ${JSON.stringify(reactElementOrRouterResult)}
You should return a React.Component always for the client side entry point.`);
} else if (shouldHydrate) {
reactHydrate(domNode, reactElementOrRouterResult as ReactElement);
} else {
reactRender(domNode, reactElementOrRouterResult as ReactElement);
reactHydrateOrRender(domNode, reactElementOrRouterResult as ReactElement, shouldHydrate);
}
}
} catch (e: any) {
Expand Down
12 changes: 0 additions & 12 deletions node_package/src/reactHydrate.ts

This file was deleted.

44 changes: 44 additions & 0 deletions node_package/src/reactHydrateOrRender.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import type { ReactElement } from 'react';
import ReactDOM from 'react-dom';
import type { RenderReturnType } from './types';

type HydrateOrRenderType = (domNode: Element, reactElement: ReactElement) => RenderReturnType;
const supportsReactCreateRoot = ReactDOM.version &&
parseInt(ReactDOM.version.split('.')[0], 10) >= 18;

// TODO: once React dependency is updated to >= 18, we can remove this and just
// import ReactDOM from 'react-dom/client';
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let reactDomClient: any;
if (supportsReactCreateRoot) {
// This will never throw an exception, but it's the way to tell Webpack the dependency is optional
// https://github.com/webpack/webpack/issues/339#issuecomment-47739112
// Unfortunately, it only converts the error to a warning.
try {
// eslint-disable-next-line global-require,import/no-unresolved
reactDomClient = require('react-dom/client');
} catch (e) {
// We should never get here, but if we do, we'll just use the default ReactDOM
// and live with the warning.
reactDomClient = ReactDOM;
}
}

export const reactHydrate: HydrateOrRenderType = supportsReactCreateRoot ?
reactDomClient.hydrateRoot :
(domNode, reactElement) => ReactDOM.hydrate(reactElement, domNode);

export function reactRender(domNode: Element, reactElement: ReactElement): RenderReturnType {
if (supportsReactCreateRoot) {
const root = reactDomClient.createRoot(domNode);
root.render(reactElement);
return root;
}

// eslint-disable-next-line react/no-render-return-value
return ReactDOM.render(reactElement, domNode);
}

export default function reactHydrateOrRender(domNode: Element, reactElement: ReactElement, hydrate: boolean): RenderReturnType {
return hydrate ? reactHydrate(domNode, reactElement) : reactRender(domNode, reactElement);
}
15 changes: 0 additions & 15 deletions node_package/src/reactRender.ts

This file was deleted.

8 changes: 0 additions & 8 deletions node_package/src/supportsReactCreateRoot.ts

This file was deleted.

12 changes: 10 additions & 2 deletions node_package/src/types/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ReactElement, Component, FunctionComponent, ComponentClass } from 'react';
import type { ReactElement, ReactNode, Component, FunctionComponent, ComponentClass } from 'react';

// Don't import redux just for the type definitions
// See https://github.com/shakacode/react_on_rails/issues/1321
Expand Down Expand Up @@ -112,6 +112,14 @@ export interface RenderResult {
renderingError?: RenderingError;
}

// from react-dom 18
export interface Root {
render(children: ReactNode): void;
unmount(): void;
}

export type RenderReturnType = void | Element | Component | Root;

export interface ReactOnRails {
register(components: { [id: string]: ReactComponentOrRenderFunction }): void;
registerStore(stores: { [id: string]: Store }): void;
Expand All @@ -126,7 +134,7 @@ export interface ReactOnRails {
clearHydratedStores(): void;
render(
name: string, props: Record<string, string>, domNodeId: string, hydrate: boolean
): void | Element | Component;
): RenderReturnType;
getComponent(name: string): RegisteredComponent;
serverRenderReactComponent(options: RenderParams): null | string | Promise<RenderResult>;
handleError(options: ErrorOptions): string | undefined;
Expand Down
44 changes: 0 additions & 44 deletions node_package/tests/supportsReactCreateRoot.test.js

This file was deleted.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
"@babel/preset-react": "^7.12.10",
"@babel/types": "^7.12.10",
"@types/jest": "^26.0.18",
"@types/node": "^14.14.11",
"@types/react": "^16.9.23",
"@types/react-dom": "^16.9.5",
"@types/turbolinks": "^5.2.0",
"@types/webpack-env": "^1.17.0",
"@typescript-eslint/eslint-plugin": "^4.10.0",
"@typescript-eslint/parser": "^4.10.0",
"babelify": "^10.0.0",
Expand Down
11 changes: 5 additions & 6 deletions spec/dummy/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6349,7 +6349,7 @@ react-is@^16.7.0:
integrity sha512-rPCkf/mWBtKc97aLL9/txD8DZdemK0vkA3JMLShjlJB3Pj3s+lpf1KaBzMfQrAmhMQB0n1cU/SUGgKKBCe837Q==

"react-on-rails@file:.yalc/react-on-rails":
version "13.0.1"
version "13.0.2"
dependencies:
"@babel/runtime-corejs3" "^7.12.5"
concurrently "^5.1.0"
Expand Down Expand Up @@ -6983,11 +6983,10 @@ sha.js@^2.4.0, sha.js@^2.4.8:
inherits "^2.0.1"
safe-buffer "^5.0.1"

shakapacker@6.4.0:
name shakapacker
version "6.4.0"
resolved "https://registry.yarnpkg.com/shakapacker/-/shakapacker-6.4.0.tgz#f49ad8bb51eb1a151b190b588ad467c6e6f62e69"
integrity sha512-bxxi7Lla3n2fnFjxVv6BtD2vcTEZIm2eb29WkJqZBmpFVT9tnwbd3XQISD9eOst6kTEJu2ruUp7dGvuEjpxwkg==
shakapacker@^6.4.0:
version "6.4.1"
resolved "https://registry.yarnpkg.com/shakapacker/-/shakapacker-6.4.1.tgz#4c3662c63acf54ecd7847f97ceb88ee1a23bfa2a"
integrity sha512-yCGqDVNP5Dz4W8wAl53M/zG6/xpvAaFItPVVlDuHyq3xCzHD2MYEBvmITNgBvvl2WU34OPcEIeVrXOWcl/MCMQ==
dependencies:
glob "^7.2.0"
js-yaml "^4.1.0"
Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1594,11 +1594,6 @@
resolved "https://registry.yarnpkg.com/@types/node/-/node-14.14.35.tgz#42c953a4e2b18ab931f72477e7012172f4ffa313"
integrity sha512-Lt+wj8NVPx0zUmUwumiVXapmaLUcAk3yPuHCFVXras9k5VT9TdhJqKqGVUQCD60OTMCl0qxJ57OiTL0Mic3Iag==

"@types/node@^14.14.11":
version "14.14.11"
resolved "https://registry.yarnpkg.com/@types/node/-/node-14.14.11.tgz#fc25a4248a5e8d0837019b1d170146d07334abe0"
integrity sha512-BJ97wAUuU3NUiUCp44xzUFquQEvnk1wu7q4CMEUYKJWjdkr0YWYDsm4RFtAvxYsNjLsKcrFt6RvK8r+mnzMbEQ==

"@types/normalize-package-data@^2.4.0":
version "2.4.0"
resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e"
Expand Down Expand Up @@ -1639,6 +1634,11 @@
resolved "https://registry.yarnpkg.com/@types/turbolinks/-/turbolinks-5.2.0.tgz#cdfd3691143ea2f452113c2a06bd12d9a88b25d6"
integrity sha512-xEgHb25lj23EaUlsU3Y4KlFH7elhlYINGSnqyn/8zmcMnenY4Idyjk/x9kw1kOoOToY3de9fD8NSwRzNc6f5nw==

"@types/webpack-env@^1.17.0":
version "1.17.0"
resolved "https://registry.yarnpkg.com/@types/webpack-env/-/webpack-env-1.17.0.tgz#f99ce359f1bfd87da90cc4a57cab0a18f34a48d0"
integrity sha512-eHSaNYEyxRA5IAG0Ym/yCyf86niZUIF/TpWKofQI/CVfh5HsMEUyfE2kwFxha4ow0s5g0LfISQxpDKjbRDrizw==

"@types/yargs-parser@*":
version "20.2.0"
resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-20.2.0.tgz#dd3e6699ba3237f0348cd085e4698780204842f9"
Expand Down