What a job ! thanks for the work on this, the serializer really needs some love.

I look through most of the implementation design it's really well done with nice interface layers.

I think there is too many extension point for a start, maybe some of them are not needed ? But we should provide an integration / extension example with api platform to have feedback on those extensions point.

Are you not afraid about the generated code implementation on maintenance burden, i think there a 2 ways on this, use this implementation or use the php-parser library but i'm not sure which one is better in terms of maintenance ?

Really hope this get accepted into symfony

is the planned changes also affect the Normalizer / Denormalizer part of the Serializer?

when using Symfony HTTPClient, it already decodes the data for me into Array,
so the Serializer just need to Denormalize the data.

Or depending on the HTTPClient (like Guzzle?) i could use the Data from a Response Stream?

Many thanks@joelwurtz! I truly think as well that this component deserves more love!

I think there is too many extension point for a start, maybe some of them are not needed ? But we should provide an integration / extension example with api platform to have feedback on those extensions point.

Yes, I can agree! I began by exposing as many extension points as I could, as it's easier to reduce them instead of adding them. But we need to define which extension point is relevant for a start.

Are you not afraid about the generated code implementation on maintenance burden, i think there a 2 ways on this, use this implementation or use the php-parser library but i'm not sure which one is better in terms of maintenance ?

Indeed, it'll reduce the added code a lot (and the maintenance burden), but at the same time complicate template generation and PHP AST optimization code as right now nodes are designed for these purposes.
I don't know about php-parser library BC promise and release process, it seems to be acceptable because it is already "opt-in" used by the translation component. But this time, it unfortunately won't be opt-in.

@Hanmac, yes, it'll affect that part. Indeed, the performance improvement relies on somehow moving the normalization/denormalization step to cache (by the computing data shape only once)

For your specific use case, you can leverage the response'sgetContent method instead to retrieve a string that you can give to the deserializer.

@mtarld my problem there:

I use the nomalizer part to read from an API
And my custom normalizer turns references into Models

Like for Products,getProduct might return this:

{  "id": "Uuidv4",  "Sector": {    "id": "Uuidv4"  }}

My normalizer notices that the Sector part is incomplete, so it automatically callsgetSector from my API client

To make the normalizer access, the current Client call, I have it access the client via Context

Would that still be possible?

I don't know about php-parser library BC promise and release process

I think they have a very strong BC policy. Rector, Phpstan, Psalm are built in top of it

@Hanmac, I think that this is the way that denormalizers should not be used.

Indeed, normalizers are meant to turn objects to array and vice versa, nothing more. Here, doing HTTP calls on the fly, or querying a database for example, will imply be a big lack of separation of concerns. The process of retrieving the actual data behind a URL must live in a custom service of yours.
Therefore, an idea instead could be to have a DTO representing the first HTTP response (with the IRI), and another one that you can fill manually with nested HTTP responses (sectors for instance).
Anyway, this must not be hidden in the denormalization process IMHO 🙂

@Hanmac, I think that this is the way that denormalizers should not be used.

Indeed, normalizers are meant to turn objects to array and vice versa, nothing more. Here, doing HTTP calls on the fly, or querying a database for example, will imply be a big lack of separation of concerns. The process of retrieving the actual data behind a URL must live in a custom service of yours. Therefore, an idea instead could be to have a DTO representing the first HTTP response (with the IRI), and another one that you can fill manually with nested HTTP responses (sectors for instance). Anyway, this must not be hidden in the denormalization process IMHO 🙂

For work, this is one of the APIs i was going to map:
https://support.korona.de/korona-cloud-api/

when load 100+ of Product with one query, i want them to load the "ModelReference" as well, like for example their sectors or ticketDefinition.

i implemented a logic in the Denormalizer to load as less objects as possible, for example:

• when multiple products references the same Sector, the Sector is only loaded once (while in the same Serializer call)
• Having Multiple products reference each other relatedProducts, they notice that when A->B->A that A is already going to be loaded, is then added later to B's property

Wow that's quite a big work, congrats@mtarld . That being said, for what it worth I have to admit I'm on the edge with this proposal as of now 😅. The perf showcase definitely makes it look awesome, but here's a few user's POV thoughts coming to my mind:

Plus, this design makes debugging hard, especially using nested normalizers.
Core normalizers make use of inheritance which leads to maintenance headaches.
Some features should've rather been left to userland and community packages [...] These add unnecessary complexity to the codebase which increases the maintenance burden.

About the why, it seems there are 2 clear intentions: improving performance, and easing design/maintenance/debugging.
The first point is clearly demonstrated, but the second is not really. My feeling going through the PR is that the proposed implementation - while being well done - is extremely hard to understand (and you seem to agree with that by stating the extension points are hard to use), and so probably to debug.
On the other hand, the current implementation is quite easy to understand, but can become hard to debug when the stack grows.
I understand there might be tradeoffs to do about simplicity when it comes to performance, but this should probably not be advertised as simplifying the design/maintenance/debugging here imho.

because it is intended to work mostly with DTOs

Why? Imho the serializer should not be intended to work with specific objects. Doc states thatThe Serializer component is meant to be used to turn objects into a specific format (XML, JSON, YAML, ...) and the other way around.

To be able to generate templates that stick as much as possible to the data model, the support of generics has been introduced.

Does that mean that if my app does not use generics, I might end up with bad generated templates? Or is it just better if I use them, i.e. no drawback in not using them compared to the current implementation?

BC & Upgrade Path

It is missing quite a big BC break here imo, dropping support for normalizers/denormalizers, while it is probably one of the most used extension points of the current implementation. Futhermore, normalizers/denormalizers are also used to be injected on their own, when you don't need to go through the full serialization. What is the suggested replacement? Could you showcase a simple normalizer/denormalizer and what it would become, would be great?
What about the other features of the current serializer, will they still work? E.g. is there a way to denormalize into an existing object? Name conversion? Etc...
Dropping some supported formats is obviously a BC break as well, but that can probably be implemented, or should be advertised.
The serializer has a straight forward and clear process, object -> normalize -> array -> encode -> string, and the other way around, which also makes it clear where are the extension points, could you showcase the workflow of your implementation in the same idea for comparison purpose? The cache miss/hit workflow you wrote is not very clear about where are the extension points, and does not make the distinction between serialization/deserialization.

Most of the time, it's a bad idea to alter the data structure depending on the
serialization configuration, it is rather recommended to use an adapted and dedicated DTO
during serialization.

That's what normalizers/denormalizers are advertised for 😅 you may need to create your own normalizer to transform an unsupported data structure. Imagine you want add, modify, or remove some properties during the serialization process. For that you'll have to create your own normalizer. But it's usually preferable to let Symfony normalize the object, then hook into the normalization to customize the normalized data.

Extension points (and concepts)

I agree on the general note that there are too many extension points, but on the other hand it is not super clear yet where they all kick in or could be useful. Maybe if you can showcase the workflow as simply as possible with all relevant extension points, could give a better clue.
Config classes: Do they really need to be immutable? I mean I understand why you want them immutable, but if you expect them to be extended in userland, you can't really guarantuee immutability then.
PropertyMetadataLoaderInterface: It seems to be the main replacement for normalizers/denormalizers, but relies on aformatter concept which is not really advertised in the PR. It feels weird to implement something about metadata to actually deal with the data. But when you think about attaching a formatter then it makes sense, so I think the concept of formatters needs to be clearly advertised. I'm not sure this cover all cases of a normalizer/denormalizer though, does it?
SplitterInterface: I think the concept needs to be better explained 😄.

Hopefully these are constructive enough questions/thoughts 😇, and congrats again for the huge work! Looking forward to see what this PR becomes!
Cheers!

@mtarld just a quick heads-up that the last paragraph (Thoughts about Type and the PropertyInfo component) in your description is currently hidden in the lastdetails tag (which describes theInstantiatorInterface for deserialization) which probably causes it to be easily overlooked.

Great. Thank you for this. Really impressed with the work you have done.

Can you please give me a simple usage example without the framework? I would like to make sure I set things up properly before I rerun my own performance tests.

To support@n-valverde's point, I wonder if this PR shouldn't be a new component of its own.

This is a better and more powerful alternative tojson_encode/decode, as the Yaml component is a powerful alternative to the YAML extension. But it's not a full replacement for the current Serializer, and it has a very different design (which is better IMHO).

It could be theJsonEncoder orJsonMarshaller component (the latter is probably better to prevent confusion with encoders from the Serializer) to use the same naming as Go. Or just theJson component to be consistent withYaml.

The current Serializer has a lot of features that will be hard to keep in your implementation (especially with a fully functional BC layer), and supporting as many features and formats as the Serializer will likely "bloat" the new code. Also, there are hundreds of projects that use the Serializer, and forcing all of them to migrate will be hard.

In most (JSON) cases, it will be possible to use the JsonEncoder component instead of the Serializer, and maybe at some point, we'll be able to feature-freeze it, and then deprecate it. But I doubt that we'll manage to do that anytime soon.

Anyway, I can't wait to get this PR merged into Symfony!

Thank you@mtarld.