-
-
Notifications
You must be signed in to change notification settings - Fork 13
Improve the Fiber and Model stream context #312
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
Conversation
17f4e18 to
0eb4aea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job
8c150ae to
12df513
Compare
12df513 to
e3adf79
Compare
d9a4e2f to
a820065
Compare
a820065 to
68f690d
Compare
* origin/main: Improve CI (#313) Add environtmnet for srr Client component's can't be functions
d1e51dc to
412a308
Compare
2651591 to
6ac0f88
Compare
| let initial_chunk_id = get_chunk_id context in | ||
| let json = client_value_to_json ~context response in | ||
| context.push initial_chunk_id (Chunk_value json); | ||
| Stream.push ~context (to_root_chunk response) |> ignore; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer to not use ignore
9f4f5ad to
e00261d
Compare
e00261d to
6383f02
Compare
| let output, subscribe = capture_stream () in | ||
| let%lwt () = ReactServerDOM.render_model ~subscribe main in | ||
| assert_list_of_strings !output | ||
| [ "0:[\"$\",\"$Sreact.suspense\",null,{\"fallback\":\"Loading...\",\"children\":\"$L2\"}]\n"; "1:\"DONE :)\"\n" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just notice that maybe we had a bug right here. Where is the ref 2 for the $L2?
8b1d06f to
94a9dde
Compare
94a9dde to
f5b9dec
Compare
Description
This PR refactors the way we handle the context for Model and Fiber. Instead of 2 different contexts for the Stream, we will have the same context type for
both.The context controls async and sync push as the index and pending, so we no longer need to worry about it. We declare the value to push and use
push/push_async, and the context will handle the rest:The concept of inversion of control was applied at
render_element_to_html andclient_values_to_json. Then we can use theclient_values_to_jsonatrender_element_to_html, which reduces the amount of code to maintain, also keeping it way simpler to understand: