procore-oss / blueprinter

Simple, Fast, and Declarative Serialization Library for Ruby
MIT License
1.14k stars 109 forks source link

[Feature Request][Rails] Render views from ActionController::API controllers #397

Closed james-em closed 8 months ago

james-em commented 9 months ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe

No

Describe the feature you'd like to see implemented

With the gem https://github.com/rails/jbuilder it is possible to keep our controllers more DRY because we don't have to explicity call render to have our JSON rendered. It instead pick a view in app/views/:controller/:action.json.jbuilder

It would be great to be able to do something similar with blueprinter.

While it's possible in most simple case to simply have

def index
  @users = User.all
  render json: UserBlueprint.render(@users)
end

There could be some use case where we want to render some specific "views" such as "normal", "extended" or whatever based on some logic that could be inside a view instead of a controller.

Feature Request

def index
  @users = User.all
end

app/views/users/index.json.erb

<%= UserBlueprint.render(@users) %>

How JBuilder does it

https://github.com/rails/jbuilder/blob/main/lib/jbuilder/railtie.rb#L12

An initializer with

  if Rails::VERSION::MAJOR >= 5
        module ::ActionController
          module ApiRendering
            include ActionView::Rendering
          end
        end

        ActiveSupport.on_load :action_controller do
          if name == 'ActionController::API'
            include ActionController::Helpers
            include ActionController::ImplicitRender
          end
        end
    end
jhollinger commented 9 months ago

Not wanting to start a meta-discussion about the meaning of "DRY", but having a single render line inside each action doesn't strike me as "duplication". Many would argue that's incidental code structure, not repeated domain knowledge. But leaving that aside for a moment...

You're suggesting BP could have an alternate mode where Blueprints live as views instead of classes? E.g. app/views/widgets/index.blueprint or something? It's an interesting idea, but it sounds like a large conceptual departure IMHO.

james-em commented 9 months ago

Thanks for the quick response! :)

Not wanting to start a meta-discussion about the meaning of "DRY", but having a single render line inside each action doesn't strike me as "duplication". Many would argue that's incidental code structure, not repeated domain knowledge. But leaving that aside for a moment...

Indeed it ain't really worth meta-discussing around the meaning of "DRY".

You're suggesting BP could have an alternate mode where Blueprints live as views instead of classes? E.g. app/views/widgets/index.blueprint or something? It's an interesting idea, but it sounds like a large conceptual departure IMHO.

The way I see it is that there is room to have "view" logic inside views instead of the controller. While most case might just be some straight 1 liner solution (a single render) it might not always be the case.

I'm kind of suggesting to have an alternate mode where we could omit the explicit render json: WidgetBlueprint.render(record) and it would fallback to the app/views/widgets/index. However, there wouldn't be a need to create a new blueprint templater to have .json.blueprint view file. We can use the built in ERB templater so the given initializer itself would be enough to provide such feature.

app/views/widgets/index.json.erb

<%= UserBlueprint.render(@users) %>

In case of ERB, it's possible we need to do this instead

<%= UserBlueprint.render(@users).html_safe %>

Rails is nice enough to pick automatically the right view based on request.format that is based on the Content-Type: application/json header. So having the following

# Controller
def index
  @widgets = Widget.all
end

# Views
app/views/widgets/index.html.erb
app/vews/widtgets/index.json.erb

The right view file is picked up by default. This feature is enabled by default in any Rails controller that inherit from ActionController::Base but it ain't by default if it inherits from ActionController::API. The given initializer enables the feature for API controller and jbuilder seems to be doing it automatically on our behalf.

My feature request is more about enabling the feature out of the box than developing a new templater like jbuilder did to support file like index.json.jbuilder

jhollinger commented 9 months ago

So you just want to call UserBlueprint.render(@users) in a view? Isn't that already possible?

james-em commented 9 months ago

So you just want to call UserBlueprint.render(@users) in a view? Isn't that already possible?

Yes and also yes already possible within an ActionController::Base controller but not ActionController::API controller unless the given initializer is setup. That's why jbuilder has it https://github.com/rails/jbuilder/blob/main/lib/jbuilder/railtie.rb#L12

jhollinger commented 8 months ago

Ok, think I understand now. I haven't used ActionController::API much, so I didn't know it lacked view functionality.

We try to keep Blueprinter framework-agnostic, so I don't think we'd want to add something like what JBuilder does here. But you could probably easily package it up into a parent class, shared module, or even your own gem.