[Feature] Allow for override of field_error_proc in the FormBuilder. Could prepare PR if the approach is reasonable

I would like to propose a way for us to define error handling in the same FormBuilder used in the forms and not to split the logic between a form builder and a configuration option as is the current approach.

This will allow us to have different error logic and css classes depending on the FormBuilder instance used for the form. This will keep the logic of generating the field and the error of the field in the same file and class.

The problem that this change addresses is the following:

In two of the platforms that we have we had to gradually migrate the user interface between internally developed styles, then bootstrap 3, then bootstrap 4, then externally developed themes based on bootstrap4, then bootstrap 5.

We do this by using the layout method of the controllers and using different form builders like

# app/controllers/articles_controller.rb

class ArticlesController < ApplicationController
  layout "based_on_bootstrap5" 
end

# app/views/articles/_form.html.erb

<%= form_with @article, builder: Bootstrap5Builder %>
  ...
<% end %>

# app/controllers/author/articles_controller.rb
class Author::ArticlesController < ApplicationController
  layout "based_on_bootstrap4"
end

# app/views/author/articles/_form.html.erb

<%= form_with @article, builder: Bootstrap4Builder %>
  ...
<% end %>

The example is simplified, but the idea is that we first migrate the Author:: where authors on our platform edit their articles. As we are happy with the new user interface that is limited only to the authors on our platform we then migrate the public views of the articles where readers could read the articles.

In the above example there are two FormBuilders, but there could be only one error handling and it is used by both.

This discussion is preceded by

and

where additional context is provided.

How the code currently works

ActionView allows us to define a custom form builder on a form_with and form_for like.

<%= form_with @article, builder: MyFormBuilder %>
  <%= f.text_field :title %>
<% end %>

The custom form builder could look like this

class MyFormBuilder < ActionView::Helpers::FormBuilder

  def text_field(method, options = {}, &block)
    ... # logic for generating the text field
  end

end

If there is an error in the activerecord object then Actionview will add the text field in a new div with a class “field_with_errors”

# actionview/lib/action_view/base.rb   
  
  cattr_accessor :field_error_proc, default: Proc.new { |html_tag, instance| content_tag :div, html_tag, class: "field_with_errors" }

This field_error_proc could be configured with

config.action_view.field_error_proc

as is visible by

# guides/source/configuring.md 

#### `config.action_view.field_error_proc`

Provides an HTML generator for displaying errors that come from Active Model. The block is evaluated within
the context of an Action View template. The default is

  Proc.new { |html_tag, instance| content_tag :div, html_tag, class: "field_with_errors" }
  

This means that the logic for generating the field and the logic for wrapping the field in an error class are in two different places

The stack trace is the following

# actionview/lib/actionview/helpers/form_helpers.rb

2003:    (field_helpers - [:label, :check_box, :radio_button, :fields_for, :fields, :hidden_field, :file_field]).each do |selector|
2004:      class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
2005:        def #{selector}(method, options = {})  # def text_field(method, options = {})
2006:          @template.public_send(               #   @template.public_send(
2007:            #{selector.inspect},               #     :text_field,
2008:            @object_name,                      #     @object_name,
2009:            method,                            #     method,
2010:            objectify_options(options))        #     objectify_options(options))
2011:        end                                    # end
2012:      RUBY_EVAL
2013:    end

Here on line 2006 we call

# actionview/lib/actionview/helpers/form_helpers.rb

1169:    def text_field(object_name, method, options = {})
1170:      Tags::TextField.new(object_name, method, self, options).render
1171:    end

This initialises a Tags::TextField and calls render

# actionview/lib/actionview/helpers/tags/text_field.rb
11: def render
12:   options = @options.stringify_keys
13:   options["size"] = options["maxlength"] unless options.key?("size")
14:   options["type"] ||= field_type
15:   options["value"] = options.fetch("value") { value_before_type_cast } unless field_type == "file"
16:   add_default_name_and_id(options)
17:   tag("input", options)
18: end

.tag on line 17 calls a method defined in ActiveModelHelpers If there are errors in the object there is a call to @template_object.instance_exec(html_tag, self, &Base.field_error_proc) on line 30.

# actionview/lib/actionview/helpers/active_model_helpers.rb

24:  def tag(type, options, *)
25:    tag_generate_errors?(options) ? error_wrapping(super) : super
26:  end
27:
28:  def error_wrapping(html_tag)
29:    if object_has_errors?
30:      @template_object.instance_exec(html_tag, self, &Base.field_error_proc)
31:    else
32:      html_tag
33:    end
34:  end

The code for generating an error is far away from the form builder. The path is FormBuilder->FormHelper->Tags::TextField->ActiveModelHelpers->config.action_view.field_error_proc

Possible improvements

My recommendation is for the FormBuilder to implement both the logic for generating a text_field and wrapping the generated text field tag in an error

Something like:

class MyFormBuilder < ActionView::Helpers::FormBuilder

  def wrap_field_with_error html_tag, field_instance
    # notice how text_field and this method that adds the css class around the text field are in the same builder. 
    @template.content_tag :div, html_tag, class: "my-form-builder-class-with-errors"
  end

  def text_field(method, options = {}, &block)
    ... # logic for generating the text field
  end

end

For this to work there could be at least two changes that could be introduced.

Injecting the FormBuilder into the ActiveModelHelpers

I thought this would be the better solution - to inject the FormBuilder instance that we are using on the form directly into active_model_helpers. Then we will have:


24:  def tag(type, options, *)
25:    tag_generate_errors?(options) ? error_wrapping(super) : super
26:  end
27:
28:  def error_wrapping(html_tag)
29:    if object_has_errors?
30:      if @form_builder.respond_to?(:wrap_field_with_error)
31:        @form_builder.wrap_field_with_error html_tag, self
32:      else 
33:        @template_object.instance_exec(html_tag, self, &Base.field_error_proc)
34:      end 
35:    else
36:      html_tag
37:    end
38:  end

Notice the use of the @form_builder instance variable.

In this way we will not break the API of field_error_proc and will allow the FormBuilders to override the logic temporarily for this call to .text_field.

The challenge with this approach is that bringing the @form_builder instance to ActiveModelHelpers will require changes in method params in FormHelpers, or Tags::TextFields, and all the other fields.

I explored this approach and it might require a rather large change and it will also change API.

Wrapping FormBuilder calls to FormHelpers

FormBuilder acts as a proxy to FormHelpers. This is visible from the comment on the class

# actionview/lib/actionview/helpers/form_helpers.rb

    # The +FormBuilder+ object can be thought of as serving as a proxy for the
    # methods in the +FormHelper+ module. This class, however, allows you to
    # call methods with the model object you are building the form for.
    #

This allows us to proxy the logic for changing the field_error_proc before and after the FormBuilder delegates to FormHelpers. Something like:

  (field_helpers - [:label, :check_box, :radio_button, :fields_for, :fields, :hidden_field, :file_field]).each do |selector|
    class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
      def #{selector}(method, options = {})  # def text_field(method, options = {})
        ensure_local_error_proc_when_set do
          @template.public_send(               #   @template.public_send(
            #{selector.inspect},               #     :text_field,
            @object_name,                      #     @object_name,
            method,                            #     method,
            objectify_options(options))        #     objectify_options(options))
        end
      end                                    # end
    RUBY_EVAL
  end

  def ensure_local_error_proc_when_set
    old_proc = ActionView::Base.field_error_proc
    begin
      if self.respond_to?(:wrap_field_with_error)
        form_builder = self
        @local_field_error_proc = Proc.new { |html_tag, field_instance| form_builder.wrap_field_with_error(html_tag, field_instance) } if @local_field_error_proc == nil
        ActionView::Base.field_error_proc = @local_field_error_proc
      end
      yield
    ensure
      ActionView::Base.field_error_proc = old_proc
    end
  end

The goal of this method is before every call to text_field to check if there is a method “wrap_field_with_error” defined in the FormBuilder and if it is then to switch the field_error_proc. At the end we return the old field_error_proc.

I saw this approach in the tests

actionview/test/actionpack/controller/render_test.rb:    old_proc = ActionView::Base.field_error_proc
actionview/test/actionpack/controller/render_test.rb:    ActionView::Base.field_error_proc = proc { |html| tag.div html, class: "errors" }
actionview/test/actionpack/controller/render_test.rb:    ActionView::Base.field_error_proc = old_proc if old_proc

This will allow us to have the two form builders with two different error wrapping logic.

class Bootstrap4FormBuilder < ActionView::Helpers::FormBuilder

  def wrap_field_with_error html_tag, field_instance
    @template.content_tag :div, html_tag, class: "one-class-with-errors"
  end

  def text_field(method, options = {}, &block)
    ... # logic for generating the text field
  end

end

class TailwindFormBuilder < ActionView::Helpers::FormBuilder

  def wrap_field_with_error html_tag, field_instance
    @template.content_tag :div, html_tag, class: "completely-different-class-with-errors-suitable-for-tailwind"
  end

  def text_field(method, options = {}, &block)
    ... # logic for generating the text field
  end

end

Pros:

  1. Engines will be able to provide their own form builders and error field wrapping logic.
  2. More than one engine could be used.
  3. Logic is easier to debug as it is in the same class
  4. API is not broken as we are still using ActionView::Base.field_error_proc if it is provide.
  5. The FormBuilders API could be introduced as a feature while the ensure_local_error_proc_when_set does not break the APIs and does not introduce changes to FormHelpers, Tags::TextField, all the other fields and ActiveModelHelper. Breaking changes, if needed, could be left for a major release.

Cons:

  1. There is an additional logic in FormBuilder#text_field.

I thought about preparing a proper PR with specs, but I wanted to get your feedback if this is a reasonable approach.

Let me know your thoughts.

@jacobdaddario tagging you as this might be of interest to you.

3 Likes

Hey! I’m so glad you’ve stuck with this. I’ve thought about this issue on and off for a while, and I always regretted that we never got it done. This has always felt like a feature that was calling out for improvement, but was bogged down by having to figure out how to untangle the long path between the code. You did a great job of laying out the key problem in your post.

At the time of opening the issue, I don’t think I had the skills to help close it, but now I think I do, and I’d love a chance to help you with this any way that I can. In thinking about this problem every now and again, I’d considered the dependency injection approach, and never ran with out of fear that it was too messy.

On the other hand, the proxy approach wasn’t something that was in my Ruby toolbox. I really like the elegance of what you’ve produced there, and I like how it piggybacks of the bit of meta-programming that already lives in FormHelper. I have two immediate concerns with it though that I frankly don’t know the answer to.

  1. Are there any other helpers that are covered by the method you’ve modified? I don’t see things like text_field, but maybe it’s dependent on one of the listed methods.
  2. Do we have any concerns about threading? I think field_error_proc is technically considered a singleton the way that it’s written. Can singletons get messed up when different threads are modifying them at the same time? I genuinely don’t know the answer to this.

In general, I really like the proxy approach, but I’m concerned about not knowing the answer to question 2. If you or anyone else has some experience with singletons and threading and know it to be safe, I’d say that second approach looks great.

Edit: I revisited the docs page for CurrentAttributes, and it’s making me think we will need to get the proxy solution to work with threading. Not 100% sure though. I will need to do some source-diving to be confident.

Edit 2: Did some source-diving. Looks like CurrentAttributes uses IsolatedExecutionState to maintain integrity between threads. Maybe we’ll need something like that?

1 Like

Thanks. These are really good questions

  1. In FormBuilder there are field helpers that are defined as
      # The methods which wrap a form helper call.
      class_attribute :field_helpers, default: [
        :fields_for, :fields, :label, :text_field, :password_field,
        :hidden_field, :file_field, :text_area, :check_box,
        :radio_button, :color_field, :search_field,
        :telephone_field, :phone_field, :date_field,
        :time_field, :datetime_field, :datetime_local_field,
        :month_field, :week_field, :url_field, :email_field,
        :number_field, :range_field
      ]

The dynamic method definition at

      (field_helpers - [:label, :check_box, :radio_button, :fields_for, :fields, :hidden_field, :file_field]).each do |selector|
        class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
          def #{selector}(method, options = {})  # def text_field(method, options = {})
            @template.public_send(               #   @template.public_send(
              #{selector.inspect},               #     :text_field,
              @object_name,                      #     @object_name,
              method,                            #     method,
              objectify_options(options))        #     objectify_options(options))
          end                                    # end
        RUBY_EVAL
      end

is using

field_helpers - [:label, :check_box, :radio_button, :fields_for, :fields, :hidden_field, :file_field]

So it is all the fields helpers from above minus the array. The result of this operation contains the text_field.

  1. I will further check.
1 Like

I looked about the thread safety and cattr_accessor is not thread safe as I understand it from https://api.rubyonrails.org/classes/Module.html#method-i-mattr_accessor. This means ActionView::Base.field_proc_error is not thread safe.

That being said this is good.

If ActionView::Base.field_error_proc is not thread safe then probably nobody is using it to dynamically change it’s value. It seems to be a configuration that is loaded once from config.action_view.field_error_proc and never changed after that and if it is, then it should not be (as it is not thread safe). The value is also set in a configuration way before it is used in a form.

What could be done is to introduce a new thread safe “variable”

# actionview/lib/action_view/base.rb   
  
  cattr_accessor :field_error_proc, default: Proc.new { |html_tag, instance| content_tag :div, html_tag, class: "field_with_errors" }

  new_thread_safe_accessor :thread_safe_field_error_proc

Then in ActiveModelHelper we can use the thread_safe_field_error_proc

@template_object.instance_exec(html_tag, self, &Base.thread_safe_field_error_proc)

Now I don’t know how exactly “new_thread_safe_accessor” will work but probably it will be close to thread_mattr_accessor at https://api.rubyonrails.org/classes/Module.html#method-i-thread_mattr_accessor

Then instead of switching ActionView::Base.field_error_proc we will be switching ActionView::Base.thread_safe_field_error_proc

def ensure_local_error_proc_when_set
    old_proc = ActionView::Base.thread_safe_field_error_proc
    begin
      if self.respond_to?(:wrap_field_with_error)
        form_builder = self
        @local_field_error_proc = Proc.new { |html_tag, field_instance| form_builder.wrap_field_with_error(html_tag, field_instance) } if @local_field_error_proc == nil
        ActionView::Base.thread_safe_field_error_proc = @local_field_error_proc
      end
      yield
    ensure
      ActionView::Base.thread_safe_field_error_proc = old_proc
    end
  end

In this way the approach will be the same with one additional layer that assigns ActionView::Base.thread_safe_field_error_proc = ActionView.field_error_proc at a certain point. Probably during initialization of ActionView::Base.

Additionally this opens the question if the field_error_proc should be a cattr_accessor or it could be something else. There are many recent changes in ActiveRecord that change cattr_accessor with class_attribute or singleton_attr_accessor like:

I am not suggesting here for the change of cattr_accessor to class_attribute in ActionView::Base as this is a rather larger change.

My suggestion is that if we keep the API of field_error_proc with can make field_error_proc more thread safe or introduce a thread_safe_field_error_proc mirror and use any of them to switch the proc based on the FormBuilder currently used.

I like the way that you’re thinking about this. I wouldn’t have considered what you just mentioned, but I really like it now that I see it laid out like that.

I guess all of that kind of begs the question, is there a missing abstraction around field_with_error in general? As you said, we totally could add in an intermediary between field_error_proc and ActionView::ActiveModelHelpers that doesn’t break the existing interface. This could allow use to maybe make a more elegant solution that doesn’t require the work-around in the proxy. That also might just be over-complicating things, and a thread-safe singleton can get the job done. I suppose that doing what you suggested is what proxy’s are for anyways.

Edit: Just did some thinking while preparing dinner. If we did a whole new abstraction for field_with_errors, could we inject ActiveModel::Errors into it as a dependency? This could allow us to expose some important error values to users by default.

there a missing abstraction around field_with_error in general

I think there is. There could be an opportunity for a full fledged interface there that we could inject an instance for.

If we did a whole new abstraction for field_with_errors , could we inject ActiveModel::Errors into it as a dependency? This could allow us to expose some important error values to users by default.

What we are currently doing in our platforms is to set the field_error_proc to do nothing and we handle all the logic in the text_field and other fields. If there are errors we add proper css classes and also the messages based on the ActiveRecord object and its errors. So there is probably an abstraction around improving field_error_proc to allow to us to show a message on the fields and the errors on those fields.

Looking at the code

# actionview/lib/action_view/helpers/active_model_helper.rb

      def error_wrapping(html_tag)
        if object_has_errors?
          @template_object.instance_exec(html_tag, self, &Base.field_error_proc)
        else
          html_tag
        end
      end

we are executing the field_error_proc in the context of the @template_object so inside of this proc we have access to the ActiveRecord that has the errors. If it is the Article instance it will be called @article and we can call @article in the proc. This is visible from the following code executed while in the proc:

#<ActionView::Base:0x0000000000b8b0>
(byebug) self.instance_variables
[:@_routes, :@_config, :@lookup_context, :@view_renderer, :@current_template, :@rendered_format, :@marked_for_same_origin_verification, :@_assigns, :@_controller, :@_request, :@_default_form_builder, :@view_flow, :@output_buffer, :@virtual_path, :@article]

So there is an opportunity to improve the abstraction, but it will require a new API to be introduced and probably to change the parameters of certain API methods.

I would really welcome feedback from the Rails Core team and which approach would be more valuable.

1 Like

In my experience, there is an upper limit threshold when considering improvements to be made to FormBuilder. If a changeset crosses that threshold and proposes to upstream a feature from a gem like simple_form, it’s likely to be rejected:

This is a reasonable stance for the core team to have, since the nuances and subtleties of gems like simple_form are anything but trivial, and any learnings or insights baked-into their codebases are likely to have been hard-won. For example, Simple Form handles this use case in its own way.

With the being said, there are still changes being made to the FormBuilder and to the field_error_proc configuration (like rails/rails#42755).

The code for generating an error is far away from the form builder.

I agree with you, the separation between the configuration-level field_error_proc and view-level FormBuilder calls can be disorienting and surprising. Personally, I’m afraid that moving a builder-specific override from the configuration-level into a class that inherits from FormBuilder isn’t enough of an improvement. Changing the declaration from a Ruby file in config/ to Ruby file in lib/ or app/.

I wonder if it’s worth capturing the error-generating block at render-time, within the form’s block. Something like:

<%# app/views/messages/new.html.erb %>
<%= form_with model: message do |form| %>
  <%= form.validation_message_template do |messages, tag| %>
    <%= tag.span messages.to_sentence, style: "color: red;" %>
  <% end %>

  <%= form.label :subject %>
  <%= form.text_field :subject %>
  <%= form.validation_message :subject %>

  <%= form.label :content %>
  <%= form.text_area :content %>
  <%= form.validation_message :content %>

  <%= form.button %>
<% end %>

The validation_message and validation_message_template methods are defined as FormBuilder extensions (lib/constraint_validations/form_builder/extensions.rb).

I’ve experimented with this pattern on a personal project, and have found it to be useful. I find declaring templating logic in Ruby instead of ERB to be extremely tedious. It’s the part of using Simple Form or custom Action View Helpers that I’m the least fond of.

One of the biggest upsides to declaring a FormBuilder’s error templating in-place is that it keeps view templating within view template files. If declaring the same block for each FormBuilder grows to be too repetitive, teams could extract their own view partials for re-use, or create an extraction or abstraction that fits their codebase.

1 Like

Thanks @seanpdoyle

I was thinking about

It is possible and gets the job done as every form could have its own error handling and there could be two different error handling(s) in the application at the same time. I thought this is a rather large change and using an additional gem like simple_form might make sense in such cases although simple_form again extracts this logic in a configuration like.

config.wrappers :with_numbers, tag: 'div', class: 'row', error_class: 'error' do |b|
  b.use :html5
  b.use :number, wrap_with: { tag: 'div', class: 'span1 number' }
  b.wrapper tag: 'div', class: 'span8' do |ba|
    ba.use :placeholder
    ba.use :label
    ba.use :input
    ba.use :error, wrap_with: { tag: 'span', class: 'help-inline' }
    ba.use :hint,  wrap_with: { tag: 'p', class: 'help-block' }
  end
end

You are right that there are changes in the field_error_proc like Execute `field_error_proc` within view by seanpdoyle · Pull Request #42755 · rails/rails · GitHub. For the last 4-5 years It is this change and

commit ead4776b82f838ee0630770d1852e8b02ac0f923
Author: neumayr <neumayr@users.noreply.github.com>
Date:   Thu Nov 9 17:37:06 2017 +0100

I wonder if the current proposed change will make sense for the Rails core team. pixeltrix shared in the github discussion that:

@thebravoman this seems like a good idea and would be a way for engines like Refinery not to stomp on an application's settings as they could create a custom form builder for its UI and just override it there. Is this something you want work on?

source: Allow for override of field_error_proc in the FormBuilder · Issue #39522 · rails/rails · GitHub

I did not focus on the Engine argument here, but it will open Rails to Engines providing different functionalities for field_error_proc. Engines like Refinery.

Is there any movement here, because I am on the verge of implementing errors? I will implement them in the text_field method. Luckily this time all the _field methods are implemented dynamically and it should not be that much overhead, but it will increase the complexity…

The field_error_proc is not enough, because I would have to do things like this:

And the biggest issue is that I cannot re-render instance.render (it goes into an infinite loop because of error_wrapping) with different classes in order to use Bootstrap 5 server side form validations:

As you can see it is set up in a way to render the field in a different div with class, the field has a class then we have the error message. I could spin Nokogiri and parse, but it’s a little too much for me at this point.

@kmitov I am starting to think that we expect way more from ActionView::Helpers::FormBuilder.

I am curious if anyone else strives to do server side form validations with turbo reloads and skip all the JS needed to achieve this. We could rack 2/3 devs and code this into rails, because this would be the second project we scratch our heads thinking where do we develop this in the most accurate place.

Most of those features are implemented in the simple_form gem. I don’t think it is responsibility of the Rails helpers to have that much logic into. They are just way to build forms, if you want to argument that adding more behavior you are able to do that by defining your own form builder.

1 Like

@rafaelfranca

I don’t think it is responsibility of the Rails helpers to have that much logic into.

Well it is actually up for debate, because I have been seeing some gaping holes in rails around accepts_nested_attributes_for and form_for which are used together. I know it is not an issue with the forms, but because there is no real solution in them we have to come up with janky solutions that work around those issues. And this for me is functionality that should be baked in into Rails helpers.

Because the error Proc is so limited you need to do this (It’s not the prettiest implementation, but the idea is the same):

  # We need to delete the top object errors because accept_nested_attributes propagates the errors to the top object.
  # Which means that we are going to display the error in the nested object filed and then on the top object with
  # custom_errors
  def get_and_delete_errors_from_top_builder_object(object_method_name)
    top_builder = options[:parent_builder] || self
    trace = [top_builder.object.class.name.split("::").last.downcase]
    while top_builder.options[:parent_builder]
      top_builder = top_builder.options[:parent_builder]
      trace << top_builder.object.class.name.split("::").last.downcase
    end

    error_key = get_error_key_for_nested_and_non_nested(object_name, object_method_name)
    top_builder.object.errors.delete(error_key)
  end

  def get_error_key_for_nested_and_non_nested(object_name, object_method_name)
    error_key = object_method_name

    nested = object_name.gsub("][", ".").gsub("_attributes", "").gsub(/\]$/, "").split("[").last

    # Because of accept_nested_attributes
    # "diets.0.breakfast" -> "diets.breakfast"
    nested = nested.split(".").select { |string| string.to_i.to_s != string }.join(".")

    if nested != object_name
      # If we are not nested form we will get
      # object_name = parent
      # The key of the error in the top object will be: "attribute"

      # If we are in nested form we will get
      # object_name = parent[child_attributes][data_attributes]
      # The key of the error in the top object will be "child.data.attribute"

      # There probably is an easier way using REGEX, but I am not going there..

      error_key = "#{nested}.#{error_key}"
    end

    error_key
  end

Which will take the error from the top most form, get the message and delete it from the object in order to show it only in the field that is for that error and not in the flash message, which is a default behaviour.

So… yeah there are some optimisations that can be made to streamline this process, but I guess we aider are the only ones that do it or the only ones that do it this way. I just go used to it by now.

Most of the functionality could be provided in a custom form builder and it should be out of Rails and in other gems or in our projects. From my view it is the error handling that should be just a little bit more extensible to allow for Rails views and helpers to be used to easily build FormBuilders.

  1. Custom form builders could be build but the error logic can not be defined and encapsulated (easily) inside in this custom form builder.
  2. Two custom builders could be defined but they can not provide (easily) their own error handling logic. They have to share a common one which defeats the purpose of two custom builders.
  3. Two engines in one rails app that both need error handling and provide their error handling can not coexist in a Rails project. This limits extensibility and engines usefulness - note - I started thinking about this problem when we were using Refinery as an engine and in the same time we had our own logic for forms. Refinery was providing error handling and we were providing error handling. We had to choose one. At the end we end up removing Refinery at all and decide not to invest in contributing to Refinery, but instead went our own way implementing logic that refinery was already providing. One of the reasons was that Rails did not had an easy, supportable way for Refinery error handling and our error handling to co-exist in the same project.

This is where my thoughts are. Does this make sense?

1 Like