Suggestion: Implement `<Model>::Persisted` opaque type to solve column nilability confusion
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 873
- Forks
- 164
- Avg merge
- 4d 27m
- Merged PRs (30d)
- 9
Description
The Problem
The tapioca compiler for ActiveRecord models makes most columns nilable, ignoring database constraints.
This is correct on paper, because when you call Model.new none of those would matter, until you eventually try to persist the record in the database.
In practice though, this is a common pitfall and cause for confusion, especially for those just getting started with Sorbet.
This is not a new problem, it has in fact been discussed in the past both here and in the Sorbet Slack workspace:
- https://github.com/Shopify/tapioca/issues/600
- https://github.com/Shopify/tapioca/issues/1342
- https://github.com/Shopify/tapioca/issues/1642
Existing Solutions
There are currently two possible paths to address this:
- The most common one is to just deal with this in the code and use
T.mustwhenever Sorbet complains about nilability and we know the record has been persisted, so the column cannot benil. - The other, less known and currently undocumented solution is to define a
StrongTypeGenerationmodule in the model to tell the compiler to generate the class following the database constraints (as explained in this comment). This will make the class work in most cases, but it won't allow you to call a non-nullable column on a newly instantiated model without throwing an error (e.g.Model.new.non_nilable_column)
The two solution seem mutually exclusive and present pros/cons to different cases, so developers will always need to weight in with additional T.must or T.unsafe to "correct" Sorbet misunderstandings.
Ideally, we'd like to teach sorbet exactly what to expect.
A proposal for an alternative solution
I recently stumbled across the concept of "opaque types" thanks to an article from Jez.
After reading the article, I began wondering if it wouldn't be possible to change the compiler so that on top of the Model class, a Model::Persisted or PersistedModel class would be generated as well.
The two classes would be identical and they'd only differ by the fact the the latter will have stricter constraints on columns, reflecting database constraints.
Then, methods signatures can be changed to return one type or the other. For example:
# app/models/user.rb
class User < ActiveRecord::Base
# == Schema Information
#
# Table name: users
#
# id :integer not null, primary key
# email :string default(""), not null
# encrypted_password :string default(""), not null
# created_at :datetime not null
# updated_at :datetime not null
end
# user.rbi
module CommonMethods
sig { returns(PersistedUser) }
def save; end
sig { returns(PersistedUser) }
def find_by; end
sig { returns(PersistedUser) }
def update; end
end
class User
include CommonMethods
sig { returns(T.nilable(Integer)) }
def id; end
sig { returns(T.nilable(String)) }
def email; end
...
end
class PersistedUser # or User::Persisted ?
include CommonMethods
sig { returns(Integer) }
def id; end
sig { returns(String) }
def email; end
...
end
cc @rafaelfranca @Morriar @KaanOzkan based on your past contributions to the topic 🙏
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Review the prior discussions in issues 600, 1342, and 1642, then compare the example app/models/user.rb and user.rbi shapes in this proposal with Tapioca's ActiveRecord RBI generation. The work is complete when the compiler can generate a persisted model type with stricter column nilability and the relevant model method signatures can use it without the existing workarounds.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rails, ruby
- Domain
- backend, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100