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

Support for React 19 #846

Draft
wants to merge 37 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4d9eeca
Add useTransition with async support
davesnx Jul 16, 2024
39189e0
Add useTransitionAsync into the interface
davesnx Jul 16, 2024
b869bd3
Add useOptimistic
davesnx Jul 16, 2024
e47a0ca
Rename use to usePromise and add useContext
davesnx Jul 16, 2024
81942e4
Move useTransitionAsync to Experimental
davesnx Jul 16, 2024
b823e5c
Add formStatus to interface
davesnx Jul 16, 2024
c4a7fdd
Leftover from removing useTransitionAsync to Experimental
davesnx Jul 16, 2024
fd7faa4
Add action_ and action_Async in ReactDOM props
davesnx Jul 16, 2024
97dd3d3
Fix reference to function on formStatus
davesnx Jul 16, 2024
e56f547
Merge branch 'main' of github.com:/reasonml/reason-react into 19
davesnx Jul 17, 2024
06b1da6
Embed FormData for now
davesnx Jul 17, 2024
9031415
Add test of Form with useOptimistic
davesnx Jul 17, 2024
52c8144
Merge branch 'main' of github.com:/reasonml/reason-react into 19
davesnx Jul 17, 2024
05d9f44
Merge branch 'main' of github.com:/reasonml/reason-react into 19
davesnx Nov 15, 2024
2190bb9
Embed FormData into ReactDOM
davesnx Nov 18, 2024
e3a97dd
Move formStatus into ReactDOM
davesnx Nov 18, 2024
ea1ff9a
Run formatter
davesnx Nov 18, 2024
a1cbb4f
Bind React.useActionState instead of ReactDOM.useFormState
davesnx Nov 18, 2024
1ad8321
Remove React.use being 'a => 'b
davesnx Nov 18, 2024
447ca53
Revert change on ReactDOM's prop 'action'
davesnx Nov 18, 2024
50bde6a
useAction state published in React.rei
davesnx Nov 18, 2024
16090ac
Remove React.use being 'a => 'b
davesnx Nov 18, 2024
e6fdb2e
Use act from 'react' inseat of react-dom/test-utils
davesnx Nov 18, 2024
ca13b5a
Snapshot with lower {}
davesnx Nov 20, 2024
563d8af
Update src/React.re
davesnx Nov 20, 2024
9ce19ea
Update src/React.re
davesnx Nov 20, 2024
5636b99
Update src/React.re
davesnx Nov 20, 2024
cb48b76
Add uri comment back on action
davesnx Nov 25, 2024
e78adcc
Add deprecations on ReactDOMTestUtils
davesnx Nov 25, 2024
f5f5579
Merge branch '19' of github.com:/reasonml/reason-react into 19
davesnx Nov 25, 2024
66cd920
Replace react dom's testing library with ReactTestingLibrary (#859)
davesnx Nov 25, 2024
08f5e19
Enable ref as valid prop (#862)
davesnx Nov 25, 2024
ea1f768
Merge branch 'main' of github.com:/reasonml/reason-react into 19
davesnx Nov 25, 2024
05ecc63
Merge branch '19' of github.com:/reasonml/reason-react into 19
davesnx Nov 25, 2024
e017f0b
Migrate custom childrens test to RTL
davesnx Nov 25, 2024
ba36dc2
Fix role for React__test
davesnx Nov 25, 2024
52e585a
Rollback change on _ppx
davesnx Nov 25, 2024
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
4 changes: 2 additions & 2 deletions ppx/test/lower.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
([@merlin.hide] ReactDOM.domProps)(
~children=
examples
|> List.map(e =>
|> List.map(e => {
let Key = e.path;
ReactDOM.jsxKeyed(
~key=Key,
Expand All @@ -120,7 +120,7 @@
),
(),
);
)
})
|> React.list,
(),
),
Expand Down
68 changes: 67 additions & 1 deletion src/React.re
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ external displayName: component('props) => option(string) = "displayName";

/* This is used as return values */
type callback('input, 'output) = 'input => 'output;
type callbackAsync('input, 'output) = 'input => Js.Promise.t('output);

/*
* Yeah, we know this api isn't great. tl;dr: useReducer instead.
Expand Down Expand Up @@ -886,6 +887,71 @@ external useDebugValue: ('value, ~format: 'value => string=?, unit) => unit =

module Experimental = {
/* This module is used to bind to APIs for future versions of React. There is no guarantee of backwards compatibility or stability. */
[@mel.module "react"] external usePromise: Js.Promise.t('a) => 'a = "use";
[@mel.module "react"] external useContext: Context.t('a) => 'a = "use";
davesnx marked this conversation as resolved.
Show resolved Hide resolved
[@mel.module "react"]
davesnx marked this conversation as resolved.
Show resolved Hide resolved
external useTransitionAsync:
unit => (bool, callbackAsync(callbackAsync(unit, unit), unit)) =
"useTransition";

/* https://es.react.dev/reference/react/useOptimistic */
davesnx marked this conversation as resolved.
Show resolved Hide resolved
[@mel.module "react"]
external useOptimistic:
Copy link
Member

Choose a reason for hiding this comment

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

can we @mel.uncurry some of these callbacks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since I don't want to push code to this branch directly, done here: #867

('state, ('state, 'optimisticValue) => 'state) =>
('state, 'optimisticValue => unit) =
davesnx marked this conversation as resolved.
Show resolved Hide resolved
"useOptimistic";

module FormData = {
/* This file is embeded since https://github.com/melange-re/melange/pull/1153 gets merged */

type t;
type file;
type blob;
type entryValue;

[@mel.new] external make: unit => t = "FormData";
[@mel.send.pipe: t] external append: (string, string) => unit = "append";
[@mel.send.pipe: t] external delete: string => unit = "delete";
[@mel.send.pipe: t] external get: string => option(entryValue) = "get";
[@mel.send.pipe: t]
external getAll: string => array(entryValue) = "getAll";
[@mel.send.pipe: t] external set: (string, string) => unit = "set";
[@mel.send.pipe: t] external has: string => bool = "has";
[@mel.send] external keys: t => Js.Iterator.t(string) = "keys";
[@mel.send] external values: t => Js.Iterator.t(entryValue) = "values";

[@mel.send.pipe: t]
external appendObject: (string, Js.t({..}), ~filename: string=?) => unit =
"append";

[@mel.send.pipe: t]
external appendBlob: (string, blob, ~filename: string=?) => unit =
"append";

[@mel.send.pipe: t]
external appendFile: (string, file, ~filename: string=?) => unit =
"append";

[@mel.send.pipe: t]
external setObject: (string, Js.t({..}), ~filename: string=?) => unit =
"set";

[@mel.send.pipe: t]
external setBlob: (string, blob, ~filename: string=?) => unit = "set";

[@mel.send.pipe: t]
external setFile: (string, file, ~filename: string=?) => unit = "set";

[@mel.send]
external entries: t => Js.Iterator.t((string, entryValue)) = "entries";
};

[@mel.module "react"] external use: Js.Promise.t('a) => 'a = "use";
type formAction;

/* https://react.dev/reference/react/useActionState */
[@mel.module "react"]
external useActionState:
(('state, FormData.t) => unit, 'state, ~permalink: bool=?) =>
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 add a unit argument at the end?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since I don't want to push code to this branch directly, done here: #867

('state, formAction, bool) =
"useActionState";
};
73 changes: 70 additions & 3 deletions src/React.rei
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,9 @@ external useReducerWithMapState:
('state, 'action => unit) =
"useReducer";

/* This is used as return values */
/* This is used as return values */
type callback('input, 'output) = 'input => 'output;
type callbackAsync('input, 'output) = 'input => Js.Promise.t('output);

[@mel.module "react"]
external useSyncExternalStore:
Expand Down Expand Up @@ -565,15 +566,81 @@ module Uncurried: {
};

[@mel.module "react"]
external startTransition: ([@mel.uncurry] (unit => unit)) => unit = "startTransition";
external startTransition: ([@mel.uncurry] (unit => unit)) => unit =
"startTransition";

[@mel.module "react"]
external useTransition: unit => (bool, callback(callback(unit, unit), unit)) =
"useTransition";

module Experimental: {
/* This module is used to bind to APIs for future versions of React. There is no guarantee of backwards compatibility or stability. */
[@mel.module "react"] external use: Js.Promise.t('a) => 'a = "use";
[@mel.module "react"] external usePromise: Js.Promise.t('a) => 'a = "use";
[@mel.module "react"] external useContext: Context.t('a) => 'a = "use";

[@mel.module "react"]
external useOptimistic:
('state, ('state, 'optimisticValue) => 'state) =>
('state, 'optimisticValue => unit) =
"useOptimistic";

[@mel.module "react"]
external useTransitionAsync:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it expected that there will be another external for useTransition? Otherwise, why the Async suffix?

Copy link
Member Author

Choose a reason for hiding this comment

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

useTransition is sync (and stable since R18) and this is Experimental.useTransitionAsync

unit => (bool, callbackAsync(callbackAsync(unit, unit), unit)) =
"useTransition";
Comment on lines +593 to +596
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prob out of scope, but now that new bindings are being added, and given they're new and experimental, it's good chance to use chatgpt to add comments to them? i got this for this one with some tweaking:

Suggested change
[@mel.module "react"]
external useTransitionAsync:
unit => (bool, callbackAsync(callbackAsync(unit, unit), unit)) =
"useTransition";
/**
* `useTransitionAsync` is a binding to React's `useTransition` hook.
*
* This hook lets you update state without blocking the UI.
*
* ### Return Value
* - `(bool, callbackAsync(callbackAsync(unit, unit), unit))`: A tuple containing:
* - `bool`: Whether the transition is pending.
* - A function to start the transition.
*
* ### Example
* ```reason
* let (isPending, startTransition) = useTransitionAsync();
*
* /* Start a transition */
* startTransition(() => {
* /* Deferred update logic */
* });
*
* if (isPending) {
* /* Show loading indicator */
* }
* ```
*/
[@mel.module "react"]
external useTransitionAsync:
unit => (bool, callbackAsync(callbackAsync(unit, unit), unit)) =
"useTransition";

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a good idea but I'd recommend doing it in a future PR. this is already quite a bit dense for revewing.


module FormData: {
/* This file is embeded since https://github.com/melange-re/melange/pull/1153 gets merged */

type t;
type file;
type blob;
type entryValue;

[@mel.new] external make: unit => t = "FormData";
[@mel.send.pipe: t] external append: (string, string) => unit = "append";
[@mel.send.pipe: t] external delete: string => unit = "delete";
[@mel.send.pipe: t] external get: string => option(entryValue) = "get";
[@mel.send.pipe: t]
external getAll: string => array(entryValue) = "getAll";
[@mel.send.pipe: t] external set: (string, string) => unit = "set";
[@mel.send.pipe: t] external has: string => bool = "has";
[@mel.send] external keys: t => Js.Iterator.t(string) = "keys";
[@mel.send] external values: t => Js.Iterator.t(entryValue) = "values";

[@mel.send.pipe: t]
external appendObject: (string, Js.t({..}), ~filename: string=?) => unit =
"append";

[@mel.send.pipe: t]
external appendBlob: (string, blob, ~filename: string=?) => unit =
"append";

[@mel.send.pipe: t]
external appendFile: (string, file, ~filename: string=?) => unit =
"append";

[@mel.send.pipe: t]
external setObject: (string, Js.t({..}), ~filename: string=?) => unit =
"set";

[@mel.send.pipe: t]
external setBlob: (string, blob, ~filename: string=?) => unit = "set";

[@mel.send.pipe: t]
external setFile: (string, file, ~filename: string=?) => unit = "set";

[@mel.send]
external entries: t => Js.Iterator.t((string, entryValue)) = "entries";
};

type formAction;

[@mel.module "react"]
external useActionState:
(('state, FormData.t) => unit, 'state, ~permalink: bool=?) =>
('state, formAction, bool) =
"useActionState";
};

[@mel.set]
Expand Down
2 changes: 1 addition & 1 deletion src/ReactDOM.re
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ type domProps = {
[@mel.optional]
acceptCharset: option(string),
[@mel.optional]
action: option(string), /* uri */
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment and the one in rei were useful imo. Or were they not accurate?

Copy link
Member

Choose a reason for hiding this comment

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

@davesnx wasn't there something about this field that bothered you a bit?

can you link us to the issue about this? I think something about action being a function for RSC or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverting this commit.

action is problematic in React 19 since it overloads it. formAction and action in this PR they are sync (while in React can be async and have different behaviour), if users want to use it they would need to use createElement + cloneElement or one of those tricks.

action: option(string),
[@mel.optional]
allowFullScreen: option(bool),
[@mel.optional]
Expand Down
2 changes: 1 addition & 1 deletion src/ReactDOM.rei
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ type domProps = {
[@mel.optional]
acceptCharset: option(string),
[@mel.optional]
action: option(string), /* uri */
action: option(string),
[@mel.optional]
allowFullScreen: option(bool),
[@mel.optional]
Expand Down
2 changes: 0 additions & 2 deletions src/ReactDOMServer.rei
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,3 @@ external renderToString: React.element => string = "renderToString";
[@mel.module "react-dom/server"]
external renderToStaticMarkup: React.element => string =
"renderToStaticMarkup";


4 changes: 2 additions & 2 deletions src/ReactDOMTestUtils.re
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ type undefined = Js.undefined(unit);

let undefined: undefined = Js.Undefined.empty;

[@mel.module "react-dom/test-utils"]
[@mel.module "react"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is not gonna be in react-dom anymore, I find it confusing to keep it in ReactDOMTestUtils. Should we move it to React for consistency / principle of less surprise?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I should probably keep both and add a deprecation

Copy link
Collaborator

Choose a reason for hiding this comment

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

idk if deprecation is necessary. A ton of other things are breaking upstream for React 19, so maybe we can just move it directly.

Copy link
Member

Choose a reason for hiding this comment

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

agree this should be moved to react.re if it's coming from require('react')

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a few "react" in ReactDOM module. I might probably propose to have this as a strict rule and align those as well in a future PR

external reactAct: ((. unit) => undefined) => unit = "act";

let act: (unit => unit) => unit =
Expand All @@ -15,7 +15,7 @@ let act: (unit => unit) => unit =
reactAct(reactFunc);
};

[@mel.module "react-dom/test-utils"]
[@mel.module "react"]
external reactActAsync: ((. unit) => Js.Promise.t('a)) => Js.Promise.t(unit) =
"act";

Expand Down
2 changes: 1 addition & 1 deletion src/dune
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
ReasonReactErrorBoundary)
(preprocess
(pps melange.ppx reason-react-ppx))
(libraries melange.dom)
(libraries melange.dom melange.js)
(modes melange))

(library
Expand Down
153 changes: 153 additions & 0 deletions test/Form__test.re
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
open Jest;
open Jest.Expect;
open ReactDOMTestUtils;
open Belt;

module FormData = React.Experimental.FormData;

/* https://react.dev/blog/2022/03/08/react-18-upgrade-guide#configuring-your-testing-environment */
[%%mel.raw "globalThis.IS_REACT_ACT_ENVIRONMENT = true"];

type message = {
text: string,
sending: bool,
key: int,
};

[@mel.send.pipe: Dom.element] external reset: unit = "reset";

let (let.await) = (p, f) => Js.Promise.then_(f, p);

module Thread = {
[@react.component]
let make = (~messages, ~sendMessage) => {
let formRef = React.useRef(Js.Nullable.null);
let (optimisticMessages, addOptimisticMessage) =
React.Experimental.useOptimistic(messages, (state, newMessage) =>
[
{text: newMessage, sending: true, key: List.length(state) + 1},
...state,
]
);

let formAction = formData => {
let formMessage = FormData.get("message", formData);
switch (formMessage) {
| Some(entry) =>
switch (Js.Types.classify(entry)) {
| JSString(text) =>
addOptimisticMessage(text);
switch (Js.Nullable.toOption(formRef.current)) {
| Some(form) => reset(form)
| None => ()
};
let.await _ = sendMessage(formData);
Js.Promise.resolve();
| _ => Js.Promise.resolve()
}
| None => Js.Promise.resolve()
};
};
<>
{{
optimisticMessages->Belt.List.map(message =>
<div key={Int.toString(message.key)}>
{React.string(message.text)}
{message.sending
? React.null
: <small> {React.string("(Enviando...)")} </small>}
</div>
);
}
->Belt.List.toArray
->React.array}
{React.cloneElement(
ReactDOM.createElement(
"form",
~props=ReactDOM.domProps(~ref=ReactDOM.Ref.domRef(formRef), ()),
[|
<input type_="text" name="message" placeholder="Hola!" />,
<button type_="submit"> {React.string("Enviar")} </button>,
|],
),
{"action": formAction},
)}
</>;
};
};

module App = {
let deliverMessage = message => {
Js.Promise.resolve(message);
};

[@react.component]
let make = () => {
let (messages, setMessages) =
React.useState(() => [{text: "¡Hola!", sending: false, key: 1}]);

let sendMessage = formData => {
let formMessage = FormData.get("message", formData);
switch (formMessage) {
| Some(message) =>
let.await entry = deliverMessage(message);
switch (Js.Types.classify(entry)) {
| JSString(text) =>
let _ =
setMessages(messages =>
[{text, sending: true, key: 1}, ...messages]
);
Js.Promise.resolve();
| _ => Js.Promise.resolve()
};
| None => Js.Promise.resolve()
};
};

<Thread messages sendMessage />;
};
};

describe("Form with useOptimistic", () => {
let container = ref(None);

beforeEach(prepareContainer(container));
afterEach(cleanupContainer(container));

test("should render the form", () => {
let container = getContainer(container);
let root = ReactDOM.Client.createRoot(container);

act(() => ReactDOM.Client.render(root, <App />));

expect(
container
->DOM.findBySelectorAndTextContent("button", "0")
->Option.isSome,
)
->toBe(true);

let button = container->DOM.findBySelector("button");

act(() => {
switch (button) {
| Some(button) => Simulate.click(button)
| None => ()
}
});

expect(
container
->DOM.findBySelectorAndTextContent("button", "0")
->Option.isSome,
)
->toBe(false);

expect(
container
->DOM.findBySelectorAndTextContent("button", "1")
->Option.isSome,
)
->toBe(true);
});
});
Loading
Loading