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

Implement mount_defaults #1152

Open
paulcsmith opened this issue May 22, 2020 · 5 comments
Open

Implement mount_defaults #1152

paulcsmith opened this issue May 22, 2020 · 5 comments
Assignees
Labels
feature request A new requested feature / option

Comments

@paulcsmith
Copy link
Member

As described here: #1049 (comment)

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

@paulcsmith paulcsmith self-assigned this May 22, 2020
@jwoertink jwoertink added the feature request A new requested feature / option label Jun 29, 2020
@stephendolan
Copy link
Member

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.

@matthewmcgarvey
Copy link
Member

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

@jwoertink
Copy link
Member

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.

@matthewmcgarvey
Copy link
Member

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

@paulcsmith
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new requested feature / option
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants