lucky icon indicating copy to clipboard operation
lucky copied to clipboard

Allow for better customization of the status and content_type

Open jwoertink opened this issue 3 years ago • 3 comments

These two things are usually the most customized when it comes to responses. For the most part, we have ways to set your custom status, but it's still a bit disjointed..

json(data)
json(data, 418)
json(data, HTTP::Status::I_AM_A_TEAPOT)
json(data, :i_am_a_teapot)

html(IndexPage)
html_with_status(IndexPage, 418)
html_with_status(IndexPage, HTTP::Status::I_AM_A_TEAPOT)
html_with_status(IndexPage, :i_am_a_teapot)

When it comes to changing the content_type, even if you wanted to add a charset, then your only option is to use send_text_response instead. If you're doing HTML and need to change the content-type even with a charset, then you're about to have a bad time and lose a lot of the type-safety Lucky gives you.

Since this all happens at the Action level, and you can only have 1 route per Action.. what if we just made this config at the action level as your default?

class IndexPage < BrowserAction

  get "/home" do
    html IndexPage
  end

  def default_status
    418
  end

  def default_content_type
    "text/html; charset=utf-9.5"
  end
end

By having these methods, you could add whatever logic you needed in case the action only returns a specific content_type or status under certain conditions...

None of this is set in stone. More just some ideas I had. The main thing to keep in mind is

  • How can we keep this type-safe with development catches
  • Keep it consistent without having an app where half your actions do one thing, and the other half do another
  • Make it easy to understand and remember. All of the different rendering methods could have compile-time catches to tell you to define this method...

jwoertink avatar Sep 23 '21 01:09 jwoertink

as per Paul's comment https://github.com/luckyframework/lucky/issues/1278#issuecomment-713838718 using a method in this way sounds like the best route

jwoertink avatar Oct 01 '21 16:10 jwoertink

Just spitballing ideas so that there's not just one possibility even if they obviously won't work:

Class Level Macro Methods

class IndexPage < BrowserAction
  status 418
  content_type "text/html; charset=utf-9.5"

  get "/home" do
    html IndexPage
  end
end

Setter Methods

class IndexPage < BrowserAction
  get "/home" do
    content_type "text/html; charset=utf-9.5"
    status 418
    html IndexPage
  end
end

Wrapping Blocks

class IndexPage < BrowserAction
  get "/home" do
    with_content_type "text/html; charset=utf-9.5" do
      with_status 418 do
        html IndexPage
      end
    end
  end
end

matthewmcgarvey avatar Oct 04 '21 19:10 matthewmcgarvey

class level not work good when you need different responses for different request types

e.g. DELETE resource request for turbo you need "text/vnd.turbo-stream.html" and status 200 for plain html you need redirect with status 302 for json you need "application/json" and status 204 No Content

skojin avatar Jan 24 '22 13:01 skojin