simple_form icon indicating copy to clipboard operation
simple_form copied to clipboard

Deprecation warning when using country_select

Open gravitystorm opened this issue 2 years ago • 2 comments

Environment

  • Ruby 2.7
  • Rails 6.1
  • Simple Form 5.1.0

Current behavior

The country_select gem introduced a deprecation warning in version 8.0.0, regarding how to set priority countries. The way that simple_form calls the gem triggers the warnings. Here is an example warning message:

app/vendor/bundle/ruby/2.7.0/gems/simple_form-5.1.0/lib/simple_form/inputs/priority_input.rb:8: warning: DEPRECATION WARNING: Setting priority countries with the 1.x syntax is deprecated. Please use the priority_countries: option.

I've found the code that causes the deprecation warning:

https://github.com/heartcombo/simple_form/blob/23205201a935c7e52c39549756e2ec2e2ec47793/lib/simple_form/inputs/priority_input.rb#L8-L9

The above is the code in simple_form that calls country_select with a separate input_priority parameter.

https://github.com/countries/country_select/blob/af59988846eee77d83be1661cb5fece5ef924158/lib/country_select/country_select_helper.rb#L4-L15

The above is the code in country_select that detects the additional parameter and emits the deprecation warning.

Expected behavior

simple_form should call country_select with a priority_countries: option, instead of a separate parameter. This has been recommended since country_select 2.0, which was released on 2014-08-10.

gravitystorm avatar Jul 04 '22 13:07 gravitystorm

@gravitystorm hey! Thanks for the report. Can you please send a PR that fixes it?

nashby avatar Jul 04 '22 14:07 nashby

@nashby It's probably a bit complicated for my skill level! The priority_input class is used for both country_select and also time_zone_select so it's not a one-line fix. It's not obvious to me (since I've not read any of the simple_form codebase before today) what the best approach to fixing this would be - perhaps to split country_select into its own class (and then that would leave priority_select being used by only one remaining input type - is that then unnecessary indirection?). Perhaps someone who is more familiar with simple_form would know the best approach, or if there is a more simple fix than renaming and refactoring 3+ classes.

gravitystorm avatar Jul 04 '22 17:07 gravitystorm