Skip to content

Conversation

davesnx
Copy link
Member

@davesnx davesnx commented Jul 28, 2023

Reference https://react.dev/blog/2022/03/29/react-v18

Fixes #742

  • Add hydrateRoot/renderRoot (move away from Experimental)
  • React.useTransition
  • React.useDeferredValue
  • React.useId
  • React.useSyncExternalStore
  • React.useInsertionEffect
  • Remove <SupenseList />
  • ReactDOM.Server.renderToPipeableStream (aliased ReactDOM.Server to ReactDOMServer)
  • React.lazy()
  • Add warnings for ReactDOM.* methods
  • Refactor tests to avoid using ReactDOM.render
  • Added React.Experimental.use

type error = {.};

[@bs.module "react-dom/server"]
external renderToPipeableStream:
Copy link
Member

Choose a reason for hiding this comment

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

this seems wrong. According to the TypeScript type definitions it should be a function of 2 arguments: a ReactNode and an options object:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f1c7be0bf694f55a4cb8b34bd1134e3d23cf21e7/types/react-dom/server.d.ts#L27

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally, I had copied my implementation from server-reason-react and forgot to bind it properly

external renderToPipeableStream: (React.element, options) => string =
"renderToPipeableStream";

let renderToPipeableStream =
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't most of these be optionally labeled arguments?

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally, fixed in 8347855 (#756)

@davesnx davesnx marked this pull request as ready for review August 4, 2023 02:14
@anmonteiro
Copy link
Member

Do you want to get this in for the upcoming release?

@anmonteiro anmonteiro force-pushed the react-18 branch 4 times, most recently from 4205df5 to be2cc2f Compare September 20, 2023 21:40
@anmonteiro
Copy link
Member

@davesnx I took the liberty to rebase this based on the latest changes in main

external useTransition: unit => (bool, callback(callback(unit, unit), unit)) =
"useTransition";

module Experimental: {
Copy link
Member

Choose a reason for hiding this comment

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

should we also include SuspenseList in this module?

Copy link
Member Author

Choose a reason for hiding this comment

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

SuspenseList got unstable by React in facebook/react#27061 and facebook/react#27056 (comment)

I'm fine keeping it but it's very useless

davesnx and others added 4 commits October 7, 2023 15:44
Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@davesnx
Copy link
Member Author

davesnx commented Oct 9, 2023

We would merge this and release under the 0.14 version!

@davesnx davesnx merged commit 1fdc7b1 into main Oct 9, 2023
@davesnx davesnx deleted the react-18 branch October 9, 2023 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use createRoot from React 18 and render from react-dom/client

2 participants