Shopify / Shopify/tapioca

Methods defined on `PrivateRelation` should return `T.self_type`

Open
#2,124 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

help-wanted
Dominant language
Ruby
Stars
873
Forks
164
Avg merge
4d 27m
Merged PRs (30d)
9

Description

I wrote a compiler for Kaminari that returns the following:

sig { T.all(PrivateRelation, Kaminari::PageScopeMethods, Kaminari::ActiveRecordRelationMethods) }
def page(num = nil); end

This type is accurate, since .page uses .extending to add methods to the relation.

This works:

User.page(5).total_count

This doesn't work:

User.page(5).all.total_count
                 ^^^^^^^^^^^ Method `total_count` does not exist on `User::PrivateRelation`

That's because the return type of for .all is PrivateRelation. We could fix this problem by using T.self_type instead, which I believe more accurately captures the way ActiveRecord works.

The way that relevant compilers are currently implemented makes this a little tricky, since the methods that are being added to the PrivateRelation are also added to the model. The methods defined on the model should be PrivateRelation.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by examining the relevant compilers that add methods to PrivateRelation and the model, then trace how the return type for .all is generated. Verify the behavior against the Kaminari example, including chaining .all and preserving the extended relation methods. Done means the generated type reflects the relation's self type without losing methods added to the model.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.