Allow explicit nil to be serialized by enums #7896
Conversation
| serialized_vals = orphaned_instances.map do |instance| | ||
| serialized_val = instance.instance_variable_get('@serialized_val') | ||
| serialized_val.equal?(UNSET) ? 'unset' : serialized_val.inspect | ||
| end.join(', ') |
There was a problem hiding this comment.
This is a bit awkward, but otherwise unset values will appear in the error message like [#<Object:0x0000000101376e08>]. That's because the @serialized_val instance variable hasn't yet been reassigned to a string representation of the constant name (which doesn't exist).
There was a problem hiding this comment.
You can fix this by declaring UNSET as
UNSET = Module.new.freezeinstead of Object.new.freeze. This way, the to_s method of the UNSET object will be implemented by way of forwarding onto UNSET.name, which because the module has been assigned to a constant, will use that constant's name.
If you make that change, you can put these lines back to what they were.
There was a problem hiding this comment.
That's much nicer, thanks for the suggestion.
thomasmarshall
force-pushed
the
enum-serialize-nil
branch
2 times, most recently
from
0ee960e
to
7696007
Compare
This commit uses an `UNSET` constant as the default `serialized_val` on `T::Enum`. This allows the runtime to understand the difference between an explicit `nil` and an unset default when serializing enum values. Sorbet already understands statically that a serialized value can be of type `NilClass`, but the runtime was serializing as a string representation based on the constant name instead.
thomasmarshall
force-pushed
the
enum-serialize-nil
branch
from
7696007
to
bc24d25
Compare
|
I force pushed to add a |
|
We have a policy of testing changes to Sorbet against Stripe's codebase before Stripe employees can see the build results here: → https://go/builds/bui_QEA383HgWOiZxN This change also has sorbet-runtime changes. Those tests: |
|
We have a policy of testing changes to Sorbet against Stripe's codebase before Stripe employees can see the build results here: → https://go/builds/bui_QFglIv5MWbPctA This change also has sorbet-runtime changes. Those tests: |
|
Is there anything else I should do for this PR? No rush, I actually don't need this fix, I just picked it up as a small and simple task to get more familiar with the Sorbet codebase 🙂 |
This commit uses an
UNSETconstant as the defaultserialized_valonT::Enum. This allows the runtime to understand the difference between an explicitniland an unset default when serializing enum values. Sorbet already understands statically that a serialized value can be of typeNilClass, but the runtime was serializing as a string representation based on the constant name instead.Motivation
Fixes #7702.
Test plan
See included automated tests.