Support mixed for getContent method
#820
Conversation
paulinevos
requested review from
Nyholm,
OskarStark and
chr-hertel
as code owners
carsonbot
changed the title
mixed for getContent methodmixed for getContent method
paulinevos
force-pushed
the
mixed-content
branch
from
3206ead to
6606004
Compare
| private PlatformInterface $platform, | ||
| private string $model, | ||
| private LoggerInterface $logger = new NullLogger(), | ||
| private readonly PlatformInterface $platform, |
There was a problem hiding this comment.
PHPStan complained about the readonly class btw. I guess this is a new rule?
| public function getId(): mixed; | ||
|
|
||
| public function getContent(): string; | ||
| public function getContent(): mixed; |
There was a problem hiding this comment.
Maybe string|object is a good alternative if we want to be a bit stricter? Because the platform normalizers also mostly assume that you're passing either strings or data classes.
There was a problem hiding this comment.
If possible I would vote to use string|object instead of mixed, WDYT @chr-hertel ?
There was a problem hiding this comment.
Yes, i would agree, currently the invoke also is explicit on the $input witharray|string|object so it would be more consistent to be explicit here as well => string|object
paulinevos
force-pushed
the
mixed-content
branch
from
6606004 to
2287f60
Compare
As the Vectorizer currently makes assumptions about having to pass string data to `PlatformInterface::invoke`, when that method can actually take about any sort of input. `Platform` implementations decide what data types they can handle through their normalizers. Therefore, it should be up to `platform` to handle types and not up to `store` to decide that it should only pass strings.
paulinevos
force-pushed
the
mixed-content
branch
from
2287f60 to
d3c8093
Compare
chr-hertel
left a comment
chr-hertel
left a comment
There was a problem hiding this comment.
Sorry for he late response, let's go with string|object here - or do we need array as well?
4 participants
As the Vectorizer currently makes assumptions about having to pass string data to
PlatformInterface::invoke, when that method can actually take about any sort of input.Platformimplementations decide what data types they can handle through their normalizers. Therefore, it should be up toplatformto handle types and not up tostoreto decide that it should only pass strings.