Support async component for React Server component in JSX v4 #6399

mununki · 2023-09-12T10:19:37Z

Resolve #6398

I've made it so that where the user defines async is considered an attribute of pvb_expr = Pexp_fun of component function. If you want the user to use it elsewhere, this will require additional implementation, let me know if you have any ideas.

mununki · 2023-09-12T10:26:34Z

@zth Can you test with it?

zth · 2023-09-12T11:30:03Z

Looks good! Let me try it in a bit. We also need the external mentioned in the issue (#6398) and autowrap the function with that so what we get back is a valid React component, for this to be usable.

mununki · 2023-09-12T11:40:40Z

Looks good! Let me try it in a bit. We also need the external mentioned in the issue (#6398) and autowrap the function with that so what we get back is a valid React component, for this to be usable.

Ah! You are correct. I missed that part.

zth · 2023-09-12T11:51:12Z

Tested and it compiles as expected 👍

mununki · 2023-09-12T11:51:42Z

There are two ways to do this

convert to React.component type using identity as mentioned in issue. This way we don't have to change any existing ppx, and I think it would make less of a breaking change to an existing project. However, I don't think it is the correct type technically, since promise<React.component> is the correct type I guess?
create a Server Component version of ppx and react-binding that extends the functionality of ppx and react-binging to match the promise<React.component> type.

In a project using Server Component, do you see many patterns being used with client components of the React.componet type? If so, I think number 2 might be a little tricky.

mununki · 2023-09-12T11:53:12Z

For example, it is not compiled

let f = a => Js.Promise.resolve(a + a)

module C = {
  @react.component
  let make = async (~a) => {
    let a = await f(a)
    <div> {React.int(a)} </div>
  }
}

let _ =
  <div>
    <C /> // Is this a common usage pattern?
  </div>

  This has type: C.props<int> => promise<Jsx.element>
  Somewhere wanted:
    React.component<C.props<int>> (defined as C.props<int> => Jsx.element)
  
  The incompatible parts:
    promise<Jsx.element> vs Jsx.element (defined as JsxC.element)

zth · 2023-09-12T12:00:20Z

I think way 1 is the best approach. React has stated that client components won't support async/await because of technical limitations, but they've changed things like this before. Conceptually, I'd say it's still a React.component and not a promise<React.component>, because React has branched out the component model to (right now only in the server context) also incorporate async/await. So I think that's fine.

@cristianoc any thoughts?

mununki · 2023-09-12T12:09:41Z

I think way 1 is the best approach. React has stated that client components won't support async/await because of technical limitations, but they've changed things like this before. Conceptually, I'd say it's still a React.component and not a promise<React.component>, because React has branched out the component model to (right now only in the server context) also incorporate async/await. So I think that's fine.

If I understand correctly.
Using an async component in the wrong context will result in a runtime error, and the user will have to deal with it. I agree.

zth · 2023-09-12T12:10:30Z

If I understand correctly. Using an async component in the wrong context will result in a runtime error, and the user will have to deal with it. I agree.

Yeah exactly. So it's the same situation as React users have in JS/TS.

mununki · 2023-09-12T12:54:31Z

If I understand correctly. Using an async component in the wrong context will result in a runtime error, and the user will have to deal with it. I agree.

Yeah exactly. So it's the same situation as React users have in JS/TS.

@zth How does it look? b0a1504

zth · 2023-09-12T12:58:42Z

@mununki works great! Completion does not work for the component, but that's something for the editor tooling.

mununki · 2023-09-13T02:23:19Z

I encounter the same issue to #6304 (comment)
Let me figure it out how to fix it little bit further.

zth · 2023-09-13T07:51:28Z

Is that one mostly about polyvariants/objects without type annotations? Or are there other cases too where it triggers/fails?

mununki · 2023-09-13T08:10:08Z

Is that one mostly about polyvariants/objects without type annotations? Or are there other cases too where it triggers/fails?

Polymorphic type seems making failed.

bruisedsamurai · 2023-09-30T08:03:13Z

Any updates guys?

zth · 2023-10-01T16:17:39Z

Me and @cristianoc had a chat about this and identity functions won't work for wrapping the full function. But, here's an approach that does seem to work:

external reactPromise: promise<React.element> => React.element = "%identity"

module AsyncComponent = {
  type props<'status> = {status: 'status}
  let make = async ({status}) => {
    switch status {
    | #on => React.string("on")
    | #off => React.string("off")
    }
  }

  let make = props => {
    make(props)->reactPromise
  }
}

let jsx = <AsyncComponent status=#on />

https://rescript-lang.org/try?version=v10.1.2&code=KYDwLsBOB2CGA2ACSxYGMwAVIHsC2AlgM7ABciADroSQDwBKqGAdMPMHsNGAHyIC8fRujCt2nbgMQAiAKQEAJlzAEwAT2kAoTXhwKAru0QBBImuhoAwvgo5oyqQG9NiROorBKuCkVoByIjBYMH0iPn5ER0Dg0PIAoJCiAF8XRHYwRDxYAGtPCNgzC0QACiiE0KSASgE+Z1dXIgB3VTQAC0RoxMjU1wAfRABiOxrEYRZAyAJoAHNi6TtpSp7EfqGAMzWRsdEJqdn5jcXllNcU1PTMnLyvHB8Ruvqs3OKqW6JKgFoeFBFsfGJgKkUmcLgArIggKS0UzmKw2OwOTqhfhDaCIAD0PE0QA

@mununki what do you think?

mununki · 2023-10-01T17:01:00Z

It seems working!

let f = a => Js.Promise.resolve(a + a)

module C = {
  @react.component
  let make = async (~a) => {
    let a = await f(a)
    <div> {React.int(a)} </div>
  }
}

let _ =
  <div>
    <C a=1 />
  </div>

module C1 = {
  @react.component
  let make = async (~status) => {
    switch status {
    | #on => React.string("on")
    | #off => React.string("off")
    }
  }
}

let jsx = <C1 status=#on />

zth · 2023-10-01T17:16:04Z

Fantastic! 😍 What do you think about this approach, good to you?

zth

All seems to be working, right? @cristianoc could have a look as well perhaps

mununki · 2023-10-01T17:29:36Z

Fantastic! 😍 What do you think about this approach, good to you?

Thanks to your hint, I realized that the PRs should be approached differently - the two PRs had similar approaches, but different problems to solve. Thank you.

mununki · 2023-10-01T17:30:59Z

The other PR I mentioned is #6304

mununki · 2023-10-01T17:34:06Z

All seems to be working, right?

Js output looks fine to me. Curious about how it runs in the runtime of RSC.

zth · 2023-10-02T07:11:31Z

@mununki it seems to work well, although I'm slightly confused by the JS output. Here's my example in code (ServerComponent.res):

type user = {
  id: string,
  name: string,
}

let getUser = async id => {
  {id, name: id}
}

@react.component
let make = async (~id) => {
  let user = await getUser(id)

  <div> {React.string(user.name)} </div>
}

And here's the emitted JS:

// Generated by ReScript, PLEASE EDIT WITH CARE

import * as JsxRuntime from "react/jsx-runtime";

async function getUser(id) {
  return {
          id: id,
          name: id
        };
}

async function make(param) {
  var user = await getUser(param.id);
  return JsxRuntime.jsx("div", {
              children: user.name
            });
}

var ServerComponent = make;

var make$1 = ServerComponent;

export {
  getUser ,
  make$1 as make,
}
/* react/jsx-runtime Not a pure module */

This is great and works as intended, but I don't see the intermediate function that casts to React.element emitted anywhere. Is that perhaps optimized away? That's great if so.

EDIT: To be clear, I've tested the above in a Next JS app using server components. And it works as expected.

mununki · 2023-10-02T13:37:44Z

This is great and works as intended, but I don't see the intermediate function that casts to React.element emitted anywhere. Is that perhaps optimized away? That's great if so.

When I looked at the rawlambda, it appears to be optimized and gone, as you said.

module React = {
  let string = s => s
}

type user = {
  id: string,
  name: string,
}

let getUser = async id => {
  {id, name: id}
}

@react.component
let make = async (~id) => {
  let user = await getUser(id)

  {React.string(user.name)}
}

(let
  (React/1002 =
     (let (string/1003 = (function s/1004 s/1004))
       (makeblock [string] string/1003))
   getUser/1008 =
     (function id/1009 (id (makeblock [id;name] id/1009 id/1009)))
   make/1026 =
     (function id/1027
       (id
         (let (user/1028 = (?await (apply getUser/1008 id/1027)))
           (apply (field:string/0 React/1002) (field:name/1 user/1028))))))
  (makeblock module/exports React/1002 getUser/1008 make/1026))

zth · 2023-10-02T14:12:57Z

That's perfect, better than I expected. Good technique to keep in mind for future use cases.

One last look from @cristianoc and then we should be good to go, right.

cristianoc · 2023-10-02T15:47:54Z

jscomp/syntax/src/react_jsx_common.ml

@@ -63,3 +63,28 @@ let removeArity binding =
    | _ -> expr
  in
  {binding with pvb_expr = removeArityRecord binding.pvb_expr}
+
+let add_async_attribute ~async (body : Parsetree.expression) =


Aren't these functions already in the file that deals with async transformations?

If you mean the files that deal with async transformation are ast_async.ml or ast_attributes.ml, the syntax module doesn't have the frontend as dependency yet. That's why I added here again.
Do you want me to add the frontend module to deps?

Oops, not able to do that due do circular deps: frontend -> common -> syntax

Looks like front-end already accesses ml:

(library (name frontend) (wrapped false) (flags (:standard -w +a-4-9-40-42-70)) (libraries common ml))

and ml is where e.g. ast_untagged_variants.ml is.

Maybe I'm missing something. syntax can access ml, but not frontend that has async transformation functions.

Perhaps my wording is confusing. Actually, this is correct: frontend <- common <- syntax

Put it where ast_untagged_variants.ml is, and change nothing else.

It's not allowed to use anything that is not already in ml, obviously.

Got it. 8126a13
I was going to put only ast_async.ml in ml, but I thought it would be nice to have ast_await.ml in the same place, so I moved it. 4d1e792

cristianoc

Looks great.
Left some comments about potential code duplication.

cristianoc · 2023-10-03T13:40:24Z

jscomp/syntax/src/react_jsx_common.ml

@@ -63,3 +63,18 @@ let removeArity binding =
    | _ -> expr
  in
  {binding with pvb_expr = removeArityRecord binding.pvb_expr}
+
+let is_async : Parsetree.attribute -> bool =


would you move this function too?

How does it look? 9d283ba
I moved is_async and is_await to ast_async and ast_await instead of moving ast_attributes.ml to ml. If we are going to move ast_attributes.ml, then other modules need to be moved too such as external_arg_spec.ml, bs_syntaxerr.ml. I think that is a bit too much.

Looks great.
Good to merge

cristianoc · 2023-10-03T13:41:10Z

jscomp/syntax/src/react_jsx_common.ml

+let is_async : Parsetree.attribute -> bool =
+ fun ({txt}, _) -> txt = "async" || txt = "res.async"
+
+let async_component ~async expr =


this should not move as it's specific to the ppx

mununki added 6 commits October 2, 2023 01:35

add tests

2fc6d8f

support async in JSX ppx

38239bc

update changelog

2c90fb4

convert async component type

efed71f

eof

8fbaee4

use asyncComponent func

a0402ae

mununki force-pushed the jsx-async branch from bb0302e to a0402ae Compare October 1, 2023 16:35

fix polymorphic type inference

e896491

zth approved these changes Oct 1, 2023

View reviewed changes

cristianoc reviewed Oct 2, 2023

View reviewed changes

mununki added 2 commits October 3, 2023 21:17

remove duplicated function

8126a13

relocate ast_await into ml

4d1e792

cristianoc reviewed Oct 3, 2023

View reviewed changes

remove duplicated is_async function

9d283ba

mununki merged commit b8c4157 into master Oct 3, 2023

mununki deleted the jsx-async branch October 3, 2023 15:29

fhammerschmidt mentioned this pull request Oct 16, 2023

Document v11 features. rescript-lang/rescript-lang.org#697

Closed

4 tasks

Support async component for React Server component in JSX v4 #6399

Support async component for React Server component in JSX v4 #6399

Conversation

mununki commented Sep 12, 2023

mununki commented Sep 12, 2023

zth commented Sep 12, 2023

mununki commented Sep 12, 2023

zth commented Sep 12, 2023

mununki commented Sep 12, 2023

mununki commented Sep 12, 2023 • edited Loading

zth commented Sep 12, 2023

mununki commented Sep 12, 2023

zth commented Sep 12, 2023

mununki commented Sep 12, 2023

zth commented Sep 12, 2023

mununki commented Sep 13, 2023

zth commented Sep 13, 2023

mununki commented Sep 13, 2023

bruisedsamurai commented Sep 30, 2023

zth commented Oct 1, 2023

mununki commented Oct 1, 2023

zth commented Oct 1, 2023

zth left a comment

Choose a reason for hiding this comment

mununki commented Oct 1, 2023

mununki commented Oct 1, 2023

mununki commented Oct 1, 2023

zth commented Oct 2, 2023 • edited Loading

mununki commented Oct 2, 2023

zth commented Oct 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristianoc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mununki commented Sep 12, 2023 •

edited

Loading

zth commented Oct 2, 2023 •

edited

Loading