heartcombo/simple_form

Rails 5.1: Relax version spec in gemspec

Closed this issue ยท 18 comments

Rails 5.1 is now out. SimpleForm requires Rails < 5.1.

simple_form (>= 3.4.0) was resolved to 3.4.0, which depends on
      actionpack (< 5.1, > 4)

Relax the gem version requirements?

Dug into this a little further. Relaxing the version is not enough... many instances of this error happen:

NoMethodError: undefined method `polymorphic_mappings' for #<MockController:0x007fa82e4e4520>
    /Users/christian/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/actionpack-5.1.0/lib/action_dispatch/routing/polymorphic_routes.rb:169:in `polymorphic_mapping'
    /Users/christian/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/actionpack-5.1.0/lib/action_dispatch/routing/polymorphic_routes.rb:130:in `polymorphic_path'
    /Users/christian/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/actionview-5.1.0/lib/action_view/helpers/form_helper.rb:472:in `apply_form_for_options!'
    /Users/christian/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/actionview-5.1.0/lib/action_view/helpers/form_helper.rb:440:in `form_for'
    /Users/christian/Projects/christiannelson/simple_form/lib/simple_form/action_view_extensions/form_helper.rb:26:in `block in simple_form_for'

I think a change in this PR is related: https://github.com/rails/rails/pull/23138/files. Though I'm not certain.

grk commented

Another issue is that Rails 5.1 moved several types from ActiveModel to ActiveRecord: decimal_without_scale, text, unsigned_integer.

simple_form has several tests for text inputs. One possible solution would be to add ActiveRecord and make the models inherit from ActiveRecord::Base, but that's a big change and I can't predict all the consequences.

rab commented

I don't know if #1488 is sufficient, but it's my attempt. The problem continues to leak out in CI due to there being no AR adapter.

@grk there are people like me who replaced ActiveRecord with Sequel. It is not a good idea to add an AR dependency.

rab commented

So, @sekrett , how would the definition of :text be found when using Sequel? I think that's going to be crucial to the overall solution as the point of moving :text from ActiveModel to ActiveRecord is that Ruby treats both :text and :string as instances of the String class.

Simple Form was working just fine for me on Rails 5.1.0.rc1 (in my specific use case), but it can't be installed with the final release because of the hard dependency.

If anybody wants to install it anyways, you can use my fork for convenience for the time being:

gem 'simple_form', github: 'elsurudo/simple_form', branch: 'rails-5.1.0'

@rab I have another idea. I want simple_form not to rely on ActiveRecord so it will be unable to know if a field is a string or a long text. It should be up to developer to decide, he can specify f.input :description, as: :text. There is a text type with no limit in PostgreSQL which is very popular, how can you know if it is long enough to become a text area? Do you really need an automatic detection of string/text field type? The same for numbers.

In Sequel there is a reflection plugin which can help detect field type. So to make everybody happy simple_form should depend on ActiveRecord and sequel-rails as well. But to me it should not be a simple_form responsibility to go so deep.

So to make everybody happy simple_form should depend on ActiveRecord and sequel-rails

except, through ROM, I'm using sequel, but not sequel rails ... hard dependencies on these libraries are not a good idea. Making use of them if they're present is fine, but don't force them on people.

It's totally fine if I'm forced to make the decision between a textarea and a input[type=text] as those are basically UI concerns anyway

rab commented

I've updated #1488 to eliminate the ActiveRecord dependency (that I'd introduced) and split the test for what is expected for the description attribute.

(I still need to look at updating documentation somewhere to describe this.)

rab commented

๐Ÿคฆโ€โ™‚๏ธ Finally have all the tests passing for all the supported versions of Rails.

Thanks to @aCandidMind for the suggestion to replace the case with a Hash.

Per the suggestions of @sekrett and @cflipse , there might be places with Rails 5.1 which will now require the addition of an explicit as: :text to maintain the use of a textarea for attributes that might now be treated implicitly as a String with an input[type=text].

So, no more automatic textarea for us? ๐Ÿ˜ข

This will make upgrade a bit hard... but it may be the best way to go without pull in database-specific dependencies.

rab commented

@fernandoluizao you'll still get an automatic textarea iff the attribute can be determined to be :text (which is now likely going to come from a database adapter via ActiveRecord). Where you still need to have a textarea, you can always add an explicit as: :text to your input. (And you can do that now, too.)

3.5.0 is out @ 5b43d10

Thanks

@rab so, if I'm using ActiveRecord my textareas will still work out of the box, without add as: :text?

murb commented

@fernandoluizao yes, <%= f.input :body %> will render you a <textarea ...></textarea> when using ActiveRecord and the column's type is :text. I have actually installed 3.5.0 and can confirm it solves the Rails 5.1 install issue and hasn't had any (noticeable) effect on my form's presentation. To the rest, thanks for the update!

Thanks @murb!