Update type to SerializationVisitorInterface?

[ro0NL]

We use a custom json visitory factory, and as such custom visitors (decorated jms_serializer.json_serialization_visitor).

I quickly realized all hell broke loose, since event handlers expect a JMS\Serializer\JsonSerializationVisitor whereas we now pass a custom SerializationVisitorInterface.

Down in the rabbit hole we had to fix/extend a bunch of handlers only to anticipate the actual contract.

See https://github.com/schmittjoh/serializer/blob/110e803463b859b918ef0f8b4759e1a356953e49/src/Handler/FormErrorHandler.php#L97 and https://github.com/FriendsOfSymfony/FOSRestBundle/blob/55a0ed94d2f1c54e22f3732736d0703c4e51be8d/Serializer/Normalizer/FormErrorHandler.php#L75

any plans to tackle this?

[GuilhemN]

We can do that. However we would need to remove the type hint and check against both JsonSerializationVisitor and SerializationVisitorInterface since the former was only introduced in JMS 2.x and JMS 1.x is still widely used (see https://packagist.org/packages/jms/serializer/stats#major/all).

[ro0NL]

our side for reference;

$ composer show jms/serializer | grep version
versions : * 3.8.0
$ composer show friendsofsymfony/rest-bundle | grep version
versions : * 2.5.0

not sure rest-bundle 2.x is still maintained tho :joy: nevertheless i think this should be solved yes :+1:

FYI our use case is serialized json with ESI includes :muscle: but im not sure where to propose such feature upstream yet, given the current ecosystem.

[GuilhemN]

Well we're still merging bug fixes, last release was in February 😉 And this one shouldn't be too complex to implement so that's fine imo

FYI our use case is serialized json with ESI includes 💪 but im not sure where to propose such feature upstream yet, given the current ecosystem.

Oh that's cool! Friendly ping to @goetas in case the subject is of interest for you :)

[ro0NL]

more ref :)

    public function visitProperty(PropertyMetadata $metadata, $data): void
    {
        if (is_string($data) && is_array($metadata->groups) && in_array(self::ESI_GROUP, $metadata->groups, true)) {
            $placeholder = "\0".md5(spl_object_hash($metadata)."\0".$data)."\0";
            $this->esiReplacements[$this->visitor->getResult($placeholder)] = $data;
            $this->visitor->visitProperty($metadata, $placeholder);

            return;
        }

        $this->visitor->visitProperty($metadata, $data);
    }

    public function getResult($data)
    {
        $result = $this->visitor->getResult($data);

        return $this->esiReplacements ? strtr($result, $this->esiReplacements) : $result;
    }

props in the ESI_GROUP are basically not JSON encoded, as we use a expression that produces the ESI include string

runs in prod =))

[ro0NL]

sorry, not relevant to us currently. Maybe i'll have a look once we update, if relevant :)