Skip to content

Conversation

@julitrows
Copy link
Contributor

@julitrows julitrows commented Nov 10, 2025

Please, disregard the branch name 😮‍💨

Changes

Add optional type checking to argument declarations

argument :jarl, type: Chiquito
argument :torpedo, type: Array, default: nil

Add optional type checking to returned values

returns MyClass, nullable: true
returns MyCollection, of: Item, nullable: true, allow_nils: false

Notes

Made them optional, since they're not the main purpose of the library, but happy to cut a 3.0.0 branch with breaking changes.

See each commit's README entries for more examples

class UserByPermissionsQuery
  include Injectable
  
  argument(:permission, type: Permission)
  dependency(:base_relation) { User.all }
  returns(ActiveRecord::Associations::CollectionProxy, of: User, nullable: false, allow_nils: false)

  def call
    base_relation.includes(:permissions).where(permissions: { id: permission.id })
  end
end

About Collection validations

I've got a bit of a mess there. Not really sure I should check for Enumerable, or just responds_to?(:each) is enough?

Maybe having an optional enumerates_with: key is cool to indicate which method is used to iterate the collection (default :each)?

returns MyCollection, of: Item, enumerates_with: :each, nullable: false, allow_nils: false

@julitrows julitrows self-assigned this Nov 10, 2025
@julitrows julitrows added enhancement New feature or request Needs Review dependencies Pull requests that update a dependency file labels Nov 10, 2025
@julitrows julitrows force-pushed the julio/ruby-3.4.7 branch 3 times, most recently from 14fcfb8 to b1a771c Compare November 10, 2025 20:47
@iovis
Copy link
Contributor

iovis commented Nov 10, 2025

Not a review, but to answer some of your questions:

Made them optional, since they're not the main purpose of the library, but happy to cut a 3.0.0 branch with breaking changes.

My idea originally was for it to be opt-in, since it has a runtime performance hit (not sure how much, but it's doing more work than before), and not everyone wants/likes strong typing.

I've got a bit of a mess there. Not really sure I should check for Enumerable, or just responds_to?(:each) is enough?

Maybe having an optional enumerates_with: key is cool to indicate which method is used to iterate the collection (default :each)?

I thought you'd do a collection.all? { it.is_a? type }. In Enumerable those are still defined in terms of :each, so I think it's a fair assumption. Still, I would assume that if someones uses of: that they're passing a collection and I would assume :each. I'd start simple and add complexity as you need it, so I'd personally keep : enumerates_with in the back burner.

@julitrows julitrows force-pushed the julio/ruby-3.4.7 branch 5 times, most recently from 9df0a6e to 29b670f Compare November 12, 2025 09:13
@julioag-rmd julioag-rmd removed the dependencies Pull requests that update a dependency file label Nov 12, 2025
@julitrows
Copy link
Contributor Author

@iovis WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Needs Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants