FEATURE: Add additional data-types array, float, dateTime - #17
Conversation
Splits the logical value carried by a metadata property from its stored string representation: MetaDataPropertyType stays the single source of truth for which concrete type each case accepts/returns, so callers type it as mixed instead of a repeated string|int|bool union that would otherwise need to grow at every call site whenever a type is added.
bwaidelich
left a comment
bwaidelich
left a comment
There was a problem hiding this comment.
Looking good! Just a few minor questions/concerns
| public MetaDataPropertyType $type, | ||
| public bool $globalScope, | ||
| public ?MetaDataPropertyUiDefinition $ui = null, | ||
| public ?array $options = null, |
There was a problem hiding this comment.
What are these? Can we add some @paramannotation with type + description
There was a problem hiding this comment.
is this related and used by the new code at all?
| return match (true) { | ||
| is_bool($coerced) => $coerced ? '1' : '0', | ||
| is_array($coerced) => json_encode($coerced, JSON_THROW_ON_ERROR), | ||
| $coerced instanceof DateTimeInterface => $coerced->format(DATE_ATOM), |
There was a problem hiding this comment.
Do we agree that DATE_ATOM is the right way to represent datetime always?
| * @throws InvalidArgumentException if the value cannot be interpreted as this type | ||
| */ | ||
| public function coerceForStorage(string|int|bool $value): string | ||
| public function coerceForStorage(mixed $value): string |
There was a problem hiding this comment.
Isn't string|int|bool|float|array|DateTimeInterface still better here?
| case integer; | ||
| case boolean; | ||
| case float; | ||
| case array; |
There was a problem hiding this comment.
do we really need to support arrays? if so I think that we should limit it to array<string|int|bool|float>
| public mixed $value, | ||
| public mixed $ownValue, | ||
| public mixed $inheritedValue, |
There was a problem hiding this comment.
mixed is the broadest type that we can use. I would prefer union so that consumers can match over all cases, e.g. if they need to serialize these
2 participants
This adds additional possible data-types and the required internal mapping for these types.
All properties are cast to string when persisted, some internal typing has been removed to allow for future additional typings to be added.