pocke / rbs_rails

Apache License 2.0
283 stars 33 forks source link

Correctly mark all Model columns as nullable #206

Closed wagenet closed 2 years ago

wagenet commented 2 years ago

This is necessary since new instances will have nil values, even for required columns.

Fixes #205

pocke commented 2 years ago

Thanks for your PR. Unfortunately, the non-nullable column is by design. I think the non-nullable column is more useful than the nullable column. .find, .create and so on are used more often than .new, so if the columns are always nullable, we need to write unnecessary null-guard (e.g. user.name or raise) everywhere.

By the way, I'm planning another approach to solve this problem. I think we can solve the problem by changing new method's returned value. For example:

# user.rbs

class User < ApplicationRecord
  interface _NewUser
    def id: () -> Integer?
    def name: () -> String?
  end

  def self.new: (...) -> User & User::_NewUser
  def id: () -> Integer
  def name: () -> String
end

# Ruby code

User.find(1).name # => String
User.new().name   # => String?

What do you think of this approach?


I have worked nothing on it yet and probably I cannot make time for it this year. Trying to implement is welcome :raised_hands:

(Probably I wrote the problem and the idea into some issue, but I couldn't find the issue :see_no_evil: )

wagenet commented 2 years ago

I think something like this probably makes sense. In TypeScript terms, a new record would be something like Partial<User>. The only catch I see here is that the record won't chance to a non-partial upon save, but I don't think there's really a solution for this in TS either.