ViewComponent / view_component

A framework for building reusable, testable & encapsulated view components in Ruby on Rails.
https://viewcomponent.org
MIT License
3.17k stars 404 forks source link

Migrate ViewComponent::Base.format to return a format instead of the variant #1973

Open BlakeWilliams opened 4 months ago

BlakeWilliams commented 4 months ago

This is related to the HTML escaping conversations, but we should use the format method to correctly indicate the type of content that the component is generating, especially since that may be utilized by https://github.com/rails/rails/pull/50623.

This is likely a breaking change, but I propose that we:

  1. Stop using variant as the format return value
  2. Implement some reasonable defaults, e.g. default to html for .html.erb, JSON for .jbuilder, etc.
  3. Document format as being overridable when necessary
  4. When implementing the call method without also implementing format, warn that we will default to HTML and escape.
camertron commented 4 months ago

This makes absolute sense to me, let's do it. /cc @jcoyne

seanpdoyle commented 4 months ago

👋 Hello, author of https://github.com/rails/rails/pull/50623 here.

In that PR's current state, I don't think there's any change to the significance of the #render_in object defining a #format method. That method pre-dated that PR, and was actually an implicit requirement of the interface. https://github.com/rails/rails/pull/50622 documented that requirement to be explicit, and also made it optional.

BlakeWilliams commented 4 months ago

@seanpdoyle 👋 thanks for commenting here too and sharing context!

I imagine we still want to make this change since the current behavior isn't exactly correct, and we'll get some benefits from implementing it. Right now I don't think format is reliable, and I don't imagine it gets much use outside of framework internals so I feel like it's a very small breaking change either way.

reeganviljoen commented 4 months ago

@BlakeWilliams @seanpdoyle I believe the request.format method is a more reliable way to get the format. It doesn't Make much sense to me to use a json format for example in an html request