attr and slot functions for LiveComponent?

[alexcastano]

Hello,

I'm working on migrating the live view dashboard to the new Phoenix.Component: phoenixframework/phoenix_live_dashboard#394

I realized that most of the code of LiveComponents are related to validations attributes or slots, ie: https://github.com/phoenixframework/phoenix_live_dashboard/blob/master/lib/phoenix/live_dashboard/components/nav_bar_component.ex#L16-L168

AFAIK we cannot use attr or slot macros in a LiveComponent, but I think it could reduce a lot of code for these components. Do you have any plans to add them? What are the difficulties you think may arise? May I help?

Thank you

[josevalim]

I think it would be awesome to migrate to actual components. We were a bit too early in our own component system :D

[alexcastano]

Yes, for sure! Now we have a much better base to build components.

I found very helpful the attr and slot macros to create functional (based on simple functions) components. Doing more complex components based on LiveComponent I'm missing these macros for default values, for validation and for default values. All these components are doing this all the time. For this reason I opened the ticket. I'm not sure the reason why these macros are only for functional components.

[connorlay]

One of the considerations when it comes to attr and slot for Live Components is how assigns can be updated with Phoenix.LiveView.send_update/3. We don't currently provide any runtime validation for declarative attributes and slots, so one could change the assigns in a Live Component at that deviate from what attr and slot declare the expected assigns to be.

[alexcastano]

I didn't think about runtime or compilation time validations. It's interesting.

Maybe for Live Components can have both validations at runtime and at compilation time. Runtime validations can be executed just before run update callback. They will be useful because we are doing this kind of validation on update/2 anyway!

So Live Components and Phoenix Components have same syntax for these macros but no exactly the same behavior.

Of course, it is a suggestion. I think it would make sense. It's strange to have these useful macros on Phoenix Components and not on Live Components.

[josevalim]

I am fine with compile-time only validation. It will provide large benefits and with type systems work the types are only going to get better.

[chrismccord]

It is more nuanced than compile/runtime. The biggest issue is the distinction between the assigns you pass to <.live_component /> and those that are actually assigned in update/2. It seems like the most valuable thing to have is the caller gets checks for the <live_component /> arguments, but you also have the assigns in the LC itself that need to be properly mapped to the render/1 function, so which is declared? Both? Also as Clay mentioned update/2 can have any number of clauses and accepts updates from both <.live_component and send_update is it's not immediately clear how attr/slot fits into LC cleanly.

[dvic]

And what about using separate macros for properties (prop) and data (data)? (like Surface)

Then prop is only for <.live_component /> while data can be for setting default assigns and runtime validation of assigns after the update callback.

Regular components could then just support the prop macro, since they don't hold any state.

[alexcastano]

I like the idea to start with only compile-time validation. It would be easier to implement because a lot of work has already been done.

Anyway, let me argue in favor of runtime validations just for future reference:

The biggest issue is the distinction between the assigns you pass to <.live_component /> and those that are actually assigned in update/2. It seems like the most valuable thing to have is the caller gets checks for the <live_component /> arguments, but you also have the assigns in the LC itself that need to be properly mapped to the render/1 function, so which is declared? Both?

IMHO I think we should declare the former because it is the real input for the component. The result of the update/2 callback seems more like an implementation detail that should not be important for the caller. But maybe there's something I'm missing.

Also as Clay mentioned update/2 can have any number of clauses and accepts updates from both <.live_component and send_update is it's not immediately clear how attr/slot fits into LC cleanly.

Sorry, I don't understand this problem clearly. At the end of the day, both ways call update/2 callback; so can't we just call a function to validate the assigns before passing to the update/2 callback? Or am I missing something here? This function can crash if validations fail. It is better to crash with an understandable and clear message than a random crash message.

I can see a small problem with the required: true validation. This validation should only be executed when mounting the LC, but not for future updates. At first sight, it seems something possible to do.

@dvic I don't have enough experience with Surface to have an opinion about it 😅 Let's see what other people think

---

If you agree with at least compile-time validation I can give it a try. I don't know if I will have enough expertise to do it, I have never looked at the LiveView code in depth.

[alexcastano]

Hello again!

Not sure if you still think it is a good or a bad idea. In Phoenix Dashboard we "solved" the situation creating "function components" that internally call to live_component: https://github.com/phoenixframework/phoenix_live_dashboard/blob/a806655bc473e42cf18ed5e8a814c37f1a5ce039/lib/phoenix/live_dashboard/page_builder.ex#L297-L339

This way, we have attributes/slots validations, default values and documentation in live components. I think this is something good. However, the problem is that if we modify the live component we can forget to update our "function component". Related code should be close. I still see the gap between function and live components.

Feel free to close the issue if you think so :)

[chrismccord]

This isn't planned at the moment because the state + render model doesn't quite fit 1:1 with declarative assigns. We may revisit this in the future