view_component-form icon indicating copy to clipboard operation
view_component-form copied to clipboard

Make `Component` suffix optional in lookup

Open boardfish opened this issue 3 years ago • 9 comments

It was previously confirmed that the Component suffix would remain a default in ViewComponent, but I'd appreciate the flexibility not to have to use it to keep in line with my existing schema for component names and namespaces.

boardfish avatar Oct 05 '21 13:10 boardfish

Hi @boardfish happy to see you here :)

How would you prefer view_component-form to handle this?

Spone avatar Oct 05 '21 14:10 Spone

Hey there! :wave: Finally getting around to solving the problem of components and forms at @raisedevs, and it looks like the ideal solution's in the works right here. Nice work!

I'd imagine either by way of a config option on the builder or ViewComponent::Form, a manual lookup, or both. I did have the loose idea of a lookup config that's something like slots in ViewComponent itself, e.g:

lookup :text_field, SomeNamespace::TextFieldComponent
lookup :text_area, -> { |params| SomeNamespace::TextFieldComponent.new(type: :text_area, **params) }

The second example in particular is interesting because I'm sure some folks will want to use the same component as a base for several different fields. You could argue that that's also achievable with inheritance as has been done here in view_component-form, so perhaps it can be skipped as a concern.

boardfish avatar Oct 05 '21 14:10 boardfish

We could draw inspiration from Policy lookup in ActionPolicy, and offer a way to customize lookup logic as well.

Spone avatar Oct 07 '21 14:10 Spone

@Spone do you think I could work on this next?

Once #160 is released this would be relatively easy to implement in configuration.

joelzwarrington avatar Apr 05 '24 17:04 joelzwarrington

@joelzwarrington we'd be happy to review your contribution to implement this option!

Spone avatar Apr 28 '24 22:04 Spone

Sounds good - I'll look at getting up a PR in the next day or so.


You previously mentioned using a similar approach to the lookup chain in Action Policy, is that still the case?

Something like this?

# in configuration...
lookup_chain = [
  lambda do |component_name, namespaces: []|
    namespaces.find do |namespace|
      "#{namespace}::#{component_name.to_s.camelize}Component".safe_constantize
    end
  end
]

and then used in ViewComponent::Form::Renderer#component_klass

def component_klass(component_name)
  @__component_klass_cache[component_name] ||= begin
    component_klass = ViewComponent::Form.configuration.lookup_chain.find do |lookup|
      lookup.call(component_name, namespaces: lookup_namespaces)
    end

    unless component_klass.is_a?(Class) && component_klass < ViewComponent::Base
      raise NotImplementedComponentError, "Component named #{component_name} doesn't exist " \
        "or is not a ViewComponent::Base class"
    end

    component_klass
  end
end

thoughts on this?

joelzwarrington avatar Apr 29 '24 18:04 joelzwarrington

hey @Spone what do you think? would you be onboard with that approach?

would be nice to be aligned before I whip something up

joelzwarrington avatar May 02 '24 16:05 joelzwarrington

👋 @joelzwarrington sorry about the late reply! Your approach sounds solid, I think you can move forward with it!

Spone avatar May 22 '24 20:05 Spone

No worries! I'll go ahead with my approach then. 👍

feel free to assign this issue to me if you'd like

joelzwarrington avatar May 24 '24 15:05 joelzwarrington