Use T::Utils.unwrap_nilable for scalar coerce_input return types#2625
Conversation
`type_for` was stripping the `T.nilable(...)` wrapper from a custom scalar's `coerce_input` return type by formatting the runtime type to a string and matching a regex. Operating on the runtime type tree is both simpler and ~8x faster on the common `T.nilable(SimpleType)` case (measured: 15.3ms vs 1.8ms across 1000 calls). Output is identical for every input the spec covers, including the new nilable-scalar case added in #2618.
paracycle
left a comment
There was a problem hiding this comment.
I hadn't even realized that return_type was already a runtime type. This makes a lot of sense!
|
@paracycle There's actually several places where we e.g. tapioca/lib/tapioca/dsl/helpers/active_model_type_helper.rb Lines 11 to 22 in ab86175 tapioca/lib/tapioca/dsl/compilers/active_model_attributes.rb Lines 123 to 124 in ab86175 I considered changing it, but the repo deals with types as strings a fair bit. I think it would more clear (and generally faster) to operate on type values, and stringify them later. What do you think? |
Overall, this is what I think: #2489 If we can move to our own type builder syntax, then we can also start generating RBS syntax if we wanted to. |
Motivation
type_forwas stripping theT.nilable(...)wrapper from a custom scalar'scoerce_inputreturn type by formatting the runtime type to a string and matching a regex. Operating on the runtime type tree is both simpler and ~8x faster on the commonT.nilable(SimpleType)case (measured: 15.3ms vs 1.8ms across 1000 calls).Output is identical for every input the spec covers, including the new nilable-scalar case added in #2618.
Implementation
Use T::Utils.unwrap_nilable || return_typeinstead.unwrap_nilablereturns nil if the type isn't nilable.Tests