5.3.0 - Custom Inputs Fail to Load

[BeatyThomas]

Environment

• Ruby [2.7.8]
• Rails [5.2]
• Simple Form [5.3.0]

Current behavior

This commit is a breaking change that prevents custom inputs in app/input from being loaded.

Expected behavior

Adding the removed snippit returns normal functionality.

[lmrodriguezr]

I seem to be experiencing this issue, but only in development, not in production. I have looked everywhere for possible configurations in the environments that could account for the difference but I can't find it anywhere. Any ideas?

[mark-dce]

I'm having the issue in both development and test environments. It appears to be related to how Rails lazy-loads classes in those environments vs. eager loading in production. My current quick-and-dirty work-around is to reference the custom input class names in an initializer, e.g.:

config/iniitilizers/simple_form.rb

# ...
SimpleForm.setup do |config|
    # ...setup stuff here..
end

# Added at the bottom of the file
# Explicitly reference the custom input classes
CurrencyInput
CustomInput
MultiValueInput
# End of File

Alternatively, it also works to locally override the SimpleForm::FormBuilder#attempt_mapping method with the code from v5.2.0:

simple_form/lib/simple_form/form_builder.rb

Lines 673 to 681 in 4851758

	def attempt_mapping(mapping, at)
	return if SimpleForm.inputs_discovery == false && at == Object
	begin
	at.const_get(mapping)
	rescue NameError => e
	raise if e.message !~ /#{mapping}$/
	end
	end

This works because it always attempts to load the class and raises an error if it's not present.

As opposed to the v5.3.0 version which never executes because the lazy loaded class constant isn't loaded, and therefore isn't defined yet:

simple_form/lib/simple_form/form_builder.rb

Lines 673 to 677 in 8f77f59

	def attempt_mapping(mapping, at)
	return if SimpleForm.inputs_discovery == false && at == Object
	at.const_get(mapping) if at.const_defined?(mapping)
	end

[dwkoogt]

I use a custom form builder that has custom fields defined in it and this change breaks it too.
It seems the change will pass the specs but you won't see it break unless you actually have custom inputs or custom form builder in your dev environment.

The intention of the calling method find_mapping attempts to map a field name to an input class and it usually finds it before getting to the attempt_mapping method. The attempt_mapping method is reached when you defined some custom field hence you have to have a custom input class or have it defined in a custom form builder. These are loaded lazily in dev so the code change that broke it made the method not do what it was supposed to do.

So, for general input types attempt_mapping not being called.

If you do not want this feature, you'd simply set inputs_discovery option to false.
You can also create a custom builder and map custom input classes using map_type class method.
Or there may a configuration option to map as well such that attempt_mapping is not called (not sure this exists).

The try catch is used because it is attempting to find a class as the method name implies.

If the argument for the change is performance, you'd only see little improvement unless you have a ton of custom inputs. So, to me the change is not really worthwhile and should be reverted.

[carlosantoniodasilva]

Released v5.3.1 reverting that and trying a slightly different approach to error checking.