-
Notifications
You must be signed in to change notification settings - Fork 132
Change load_async return type to self
#997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change load_async return type to self
#997
Conversation
Update `load_async` to return `self` instead of the explicit `ActiveRecord::Relation[Model]`. While both represent the same runtime object, RBS treats them as distinct types from `Model::ActiveRecord_Relation` (returned by methods like `where` or `all`). This discrepancy causes type mismatches and forces unnecessary type distinctions during method chaining or when defining signatures. Changing it to `self` ensures consistency with other relation methods.
|
@rhiroe Thanks for your contribution! Please follow the instructions below for each change. Available commandsYou can use the following commands by commenting on this PR.
|
|
@dak2 Since you were the one who originally added the types for this method, I believe this change will affect your work. I'd love to get your thoughts on this. |
Exactly.
In my case, the impact was minimal because I only used it partially. |
|
/merge |
|
This PR is not approved yet by the reviewers. Please get approval from the reviewers. See the Actions tab for detail. |
ksss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APPROVE
|
/merge |

Problem
Currently,
load_asyncis defined to returnActiveRecord::Relation[Model]. This creates a technical inconsistency because most other relation methods (such aswhere,all, andorder) returnModel::ActiveRecord_Relation.Although these represent the same runtime object, RBS treats them as distinct types. This forces users to unnecessarily distinguish between them when handling return values or defining method signatures, even though consistency with other common methods is expected.
Concerns
This is a breaking change. It will cause type mismatches in projects where other RBS definitions are written assuming
load_asyncreturnsActiveRecord::Relation[Model]. However, I believe unifying the return type with the standardModel::ActiveRecord_Relationsignificantly improves DX and reduces confusion across the RBS ecosystem.What are your thoughts on this change?