Skip to content

Conversation

@pedrobslisboa
Copy link
Collaborator

@pedrobslisboa pedrobslisboa commented Oct 6, 2025

Description

This PR allows Client Components to have Async Components on their props.
To do so, I also had to handle the way we render inside client-props.

Solution

  1. We were raising when Async_component was found on a Client Component.
| Async_component (_, _component) ->
    (* async components can't be interleaved in client components, for now *)
    raise
      (Invalid_argument
         "async components can't be part of a client component. This should never raise, the ppx should catch it")

Now, we treat it as a normal Async_component:

| Async_component (_, component) ->
    let%lwt element = component () in
    client_to_html ~fiber element
  1. If any Suspense with promise were find on a client component, we pushed it twice. That was happening because were calling the client_to_html to render the html and render_element_to_html while handling the lwt_props, but bothrender_element_to_html and client_to_html pushed the resolved html to the context.
    The solution was to add a flag skip_push_html. When handling client component props inside, we don't want to push any HTML.
| Client_component { import_module; import_name; props; client } ->
    let context = Fiber.get_context fiber in
    let lwt_props =
      Lwt_list.map_p
        (fun (name, value) ->
          match (value : React.client_value) with
          | Element element ->
-                let%lwt _html, model = render_element_to_html ~fiber element in
+               let%lwt _html, model = render_element_to_html  ~skip_push_html ~fiber element in
              Lwt.return (name, model)
     (* ... *) in
-      let lwt_html = client_to_html ~fiber (client ()) in
+      let lwt_html = client_to_html ~skip_push_html ~fiber (client ()) in

@pedrobslisboa pedrobslisboa requested a review from davesnx October 6, 2025 21:36
@pedrobslisboa pedrobslisboa self-assigned this Oct 6, 2025
* origin/main:
  fix: adjust doc link on tests
@pedrobslisboa pedrobslisboa marked this pull request as draft October 11, 2025 17:43
@pedrobslisboa
Copy link
Collaborator Author

This PR will be affected by this change: #312
No need for review RN.

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.

2 participants