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

Update docs on nested Kino.start_child/1 #452

Merged
merged 3 commits into from
Jul 3, 2024
Merged

Update docs on nested Kino.start_child/1 #452

merged 3 commits into from
Jul 3, 2024

Conversation

jonatanklosko
Copy link
Member

🐈

Comment on lines -219 to -220
> frame = Kino.Frame.new()
> {:ok, assign(ctx, frame: frame)}
Copy link
Member Author

Choose a reason for hiding this comment

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

This example is confusing, we create a frame in Kino.JS.Live, but there is no obvious way how we would use that frame (since Kino.JS.Live itself is already about managing JS rendering). If we want a frame to render updates into, as we do in the dbg pipeline, it is actually more natural to create the frame upfront and pass as an argument.

I think it's more about creating kinos like Kino.DataTable.new() and rendering into a provided frame, and it's also more about GenServer, because it's a rare case for Kino.JS.Live.

lib/kino.ex Outdated Show resolved Hide resolved
Co-authored-by: José Valim <[email protected]>
@jonatanklosko jonatanklosko merged commit bca0dfb into main Jul 3, 2024
1 check passed
@jonatanklosko jonatanklosko deleted the jk-docs branch July 3, 2024 09:38
> that you cannot use functions such as `Kino.DataTable.new/1` in
> `c:GenServer.init/1`. If you need to do that, you must either
> create the kinos beforehand and pass in the `GenServer` argument,
> or create them in `c:GenServer.handle_continue/2`.
Copy link
Member

Choose a reason for hiding this comment

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

Ohhh, didn't know it was possible to do that inside handle_continue ! 💡

Choose a reason for hiding this comment

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

@hugobarauna I've been trying to find an example of this. Do you happen to know of an example smartcell that uses handle_continue/2 to start a child kino?

The issue I'm running into is with Kino..SmartCell.Server.validate_init_opts!/2 requiring init options.

Copy link
Member Author

Choose a reason for hiding this comment

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

@timgremore I don't think Kino.start_child is relevant for smart cell implementation. The idea is that you use Kino.start_child to start processes in cell code that gets evaluated, so that Livebook knows that it should kill the process before the cell is evaluated again. On the other hand, a smart cell starts and runs in the background, regardless of what gets evaluated (it's just a server process for driving the UI and code generation). If you need the smart cell to start a background process, you can just use start_link.

Do you happen to know of an example smartcell that uses handle_continue/2 to start a child kino?

By "child kino" do you mean like Kino.DataTable.new(...) or similar? Those kinos are generally supposed to be used for cell outputs only. What is your use case? :)

Choose a reason for hiding this comment

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

@jonatanklosko I think I follow the distinction you're explaining. My application involves a :gen_statem implementation that I'm representing in UI. I attempted to start my state machine with something like (this the child kino I referenced above):

Kino.start_child!(%{
  start: {MyStateServer, :start_link, []},
  id: MyStateServer
})

I had thought handle_continue/2 would be an appropriate spot for that call. But I understand your point about that being more suitable for cell code.

Seems start_link is more fitting for what I'm trying to achieve but, if I understand correctly, I'll need something else to start :gen_statem. Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

If I understand correctly that you are defining a smart cell and starting the :gen_statem server from the smart cell, you can just do MyStateServer.start_link([]) in the smart cell init and it should be all good.

Choose a reason for hiding this comment

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

That's it! Thank you @jonatanklosko for helping me sort that out

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.

4 participants