refactor(core): adjusted the types to properly extract the hidden and optional indicators #5915
Conversation
|
I enumerated all the possible combinations of Hidden, IType and null, and I can't for the life of me make all the cases pass. No matter how I adjust the types, there's always something that fails. The branch in its current state is what I managed to do as a way to enable some way for a property to be both hidden and nullable... Namely, There's also the fact that nullability may be runtime only or raw only... Which works in this branch, but when only the raw is nullable, Hidden needs to be at the Serialized type only for the nullability to be honored only when converting. |
124e1d9 to
6370ead
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5915 +/- ##
========================================
Coverage 99.79% 99.79%
========================================
Files 264 264
Lines 18825 18825
Branches 4685 4116 -569
========================================
Hits 18786 18786
Misses 39 39 ☔ View full report in Codecov by Sentry. |
6370ead to
a02fed6
Compare
|
@B4nan Any ideas on this one? In #5849, I enabled the entity generator to handle mixed null output, but when null is combined with Hidden, it's still a mess. We could merge this as is, and I can adjust the entity generator to output |
… optional indicators
a02fed6 to
8e9d729
Compare
|
I think you are too concerned with edge cases nobody even reported, I wouldn't bother with custom types that can remap null to a value now. What else is this trying to solve? The changes are quite complex (which I am not very happy to see, as it can lead to perf issues). |
Perhaps you're right, but I can imagine cases where I'd consider doing exactly that in a custom type 😅. Most notably, if I had a non-nullable column, and I wanted to take in NULL to mean "figure this out automatically for me based on the request context" and encode said default logic that way. This allows me to keep the create() call simple, rather than require explicit call to a constructor that would create said default value. I'm thinking f.e. a user account where the "password" field is optional, and if not supplied, would simply mean you can only login via email link. The custom type would generate hash over an empty string when NULL is given at runtime.
IIRC, with or without this PR, I can't get to be optional, while is optional, but not hidden. And IType is merely complicating stuff further... With this PR in place, the ugly workaround works, but I don't like that either. |
I wouldn't bother with that.
And if this doesn't work for the
Only if you consider null as a valid value for it, which I wouldn't care about really. I believe we never hydrate nulls into custom types. |
No description provided.