activemodel: ActiveModel::Errors#add accepts String and Proc for type #717
Conversation
|
@sanfrecce-osaka Thanks for your contribution! Please follow the instructions below for each change. Available commandsYou can use the following commands by commenting on this PR.
|
sanfrecce-osaka
force-pushed
the
fix-activemodel-errors-add
branch
from
d783e3c to
0b39c22
Compare
… type The behavior of passing String or Proc for type should be allowed. FYI: It is documented in 6.1, but It is already available in 6.0. cf. rails/rails@fcd1e41#diff-4deafad6fefcf6aa6626a1f7e3cd0ca1cf95e358f55a10a69e02be84d926bb31R368-R374 cf. rails/rails@d9011e3#diff-4deafad6fefcf6aa6626a1f7e3cd0ca1cf95e358f55a10a69e02be84d926bb31R370 Also, the name of the second argument was changed from `message` to `type` in 6.1.
sanfrecce-osaka
force-pushed
the
fix-activemodel-errors-add
branch
from
0b39c22 to
fe5bb2d
Compare
gems/activemodel/6.0/activemodel.rbs
Outdated
| # # => {:base=>["either name or email must be present"]} | ||
| # person.errors.details | ||
| # # => {:base=>[{error: :name_or_email_blank}]} | ||
| def add: (untyped attribute, ?::Symbol | ::String | ^(untyped base, ::Hash[untyped, untyped]) -> (::Symbol | ::String) type, ?::Hash[untyped, untyped] options) -> void |
There was a problem hiding this comment.
What makes you think return type should be void instead of ActiveModel::Error?
There was a problem hiding this comment.
Thank you for your feedback.
ActiveModel::Error was added on 6.1.
On 6.1 or later, ActiveModel::Errors#add returns ActiveModel::Error.
On the other hand, ActiveModel::Errors#add returns messages array on rails 6.0, because last evaluated is Array#<<.
cf. https://github.com/rails/rails/blob/v6.0.0/activemodel/lib/active_model/errors.rb#L311-L322
Currently in gem_rbs_collection, activemodel type definitions are defined 6.0 and 7.0, but not 6.1.
cf. #615
So I fixed them on 174b6ce
…o 6.0 and 7.0 ActiveModel::Error was added on 6.1. cf. rails/rails@ef68d3e On 6.1 or later, ActiveModel::Errors#add returns ActiveModel::Error. On the other hand, `ActiveModel::Errors#add` returns messages array on rails 6.0, because last evaluated is `Array#<<`. cf. https://github.com/rails/rails/blob/v6.0.0/activemodel/lib/active_model/errors.rb#L311-L322 Currently in gem_rbs_collection, activemodel type definitions are defined 6.0 and 7.0, but not 6.1. cf. ruby#615 So I divide type definition of ActiveModel::Errors#add to 6.0 and 7.0.
sanfrecce-osaka
force-pushed
the
fix-activemodel-errors-add
branch
from
4981ed6 to
174b6ce
Compare
|
Thanks for your review, @ksss! @sanfrecce-osaka, @ksss This PR is ready to be merged. |
…ors-add # Conflicts: # gems/activemodel/6.0/activemodel-6.0.rbs
|
/merge |
|
This PR is not approved yet by the reviewers. Please get approval from the reviewers. See the Actions tab for detail. |
|
Sorry, I resolved conflict. Please re-review 🙏 🙏 🙏 |
|
Thanks for your review, @ksss! @sanfrecce-osaka, @ksss This PR is ready to be merged. |
|
/merge |
The behavior of passing String or Proc for type should be allowed.
FYI: It is documented in 6.1, but It is already available in 6.0.
cf. rails/rails@fcd1e41#diff-4deafad6fefcf6aa6626a1f7e3cd0ca1cf95e358f55a10a69e02be84d926bb31R368-R374
cf. rails/rails@d9011e3#diff-4deafad6fefcf6aa6626a1f7e3cd0ca1cf95e358f55a10a69e02be84d926bb31R370
Also, the name of the second argument was changed from
messagetotypein 6.1.