lucky icon indicating copy to clipboard operation
lucky copied to clipboard

Implement `mount_defaults`

Open paulcsmith opened this issue 5 years ago • 5 comments
trafficstars

As described here: https://github.com/luckyframework/lucky/issues/1049#issuecomment-611689438

The idea is that common needs can be passed in automatically. Stuff like context, current_user

paulcsmith avatar May 22 '20 20:05 paulcsmith

Just ran into an example where this would have been really fantastic to have access to:

class Filters::Form < BaseComponent
  needs operation : SaveFilter
  needs route : Lucky::RouteHelper
  needs button_text : String

  def render
    form_for route, class: "divide-y divide-gray-200" do
      div class: "px-4 py-5 sm:p-6 space-y-4" do
        mount Filters::FormFields, operation
      end

      div class: "px-4 py-4 sm:px-6" do
        mount UI::Button, &.submit button_text, data_disable_with: button_loading_text
      end
    end
  end

  # Lots of chaining here, but a pretty simple result.
  #
  # Example:
  # ```
  # "Create the thing" => "Creating..."
  # "Create" => "Creating..."
  # "Update blog post" => "Updating..."
  # "Update" => "Updating..."
  # ```
  private def button_loading_text
    button_text.split.first.rchop + "ing..."
  end
end

Because of a lack of @context, the forgery protection can't be initialized on the form.

stephendolan avatar Apr 20 '21 21:04 stephendolan

Before we go down this path, we might want to reference how viewcomponent does this

The component has a render_in method that is passed the context so instead of creating the component with the context, it raises an error if the context is accessed before it's set. I believe that would be very helpful in Lucky so that components could be used outside of html response rendering and have certain boundaries

matthewmcgarvey avatar Apr 20 '21 22:04 matthewmcgarvey

This would be used for a lot more than just context though. In my case, I always need access to the current_site, and in most cases context. So for all of my components, I'm having to always do

mount Header, current_site: current_site, context: context, current_member: current_member

mount List, things: things, current_site: current_site, context: context, current_member: current_member

mount Footer, current_site: current_site, context: context

I think mount_defaults would actually work similar to expose. This could be something that goes in to the Page like:

abstract class MainLayout
  needs current_site : Site
  needs context : HTTP::Server::Context
  needs current_member : Member
end

class Members::IndexPage < MainLayout
  mount_defaults current_site, context

  def content
     mount List, current_member: current_member
  end
end

Or this would go in the BaseComponent

abstract class BaseComponent < Lucky::Component
  mount_defaults current_site, context
end

On the page, we might already know the types and methods exist, so maybe it's possible. On the component level, it would be a lot harder.

jwoertink avatar Apr 20 '21 22:04 jwoertink

Could current_site be monkey patched onto HTTP::Server::Context? "Global request state" is kind of what that class seems like it's for

matthewmcgarvey avatar Apr 21 '21 01:04 matthewmcgarvey

Could current_site be monkey patched onto HTTP::Server::Context? "Global request state" is kind of what that class seems like it's for

I think in a dynamically typed language this is definitively where I'd put it. In Elixir this kind of thing lives in a context that is pass around to everything.

This may not work as well in Crystal though since you'd either have to:

  • Make some properties nilable since something like current_user may not always be available. Then you'd have to check for nil everywhere or use not_nil! and get potential runtime errors
  • Make different contexts SignedInContext, GuestContext, which you'd still need to pass and define in your components

I think mount_defaults would be better suited to Lucky/Crystal since you could get the type-safety. It is not perfect, but I think I'd make the tradeoff of type-safety. I don't love the name though. Definitely open to something else. Since the idea is that mount_defaults is used infrequently (in a base component for example), maybe something wordier is better

  • defaults_on_mount
  • include_on_mount

Not sure. Definitely open to ideas

paulcsmith avatar Jun 15 '21 15:06 paulcsmith