Spin-off from #3029897-23: [PP-1] Do not overload Symfony's normalize() method to return objects; define a new interface for that..1. In that comment, @Wim Leers wrote:

But it is less clear/explicit which normalizers allow leaving the JSON:API normalizers and "reach into core normalizers": the @FieldType-level normalizers of JSON:API do this, to get the @DataType-level normalizers.

In other words, currently, the JSON:API serializer delegates to the core serializer as a fallback whenever there isn't a JSON:API normalizer for the passed in data object. But in practice, we only want that delegation for field item properties.

If we indeed want to control the delegation to only field item properties, a way to do that is to add a JSON:API FieldItemPropertyNormalizer. It can implement supportsNormalization() to return TRUE only if $data->getParent() is a FieldItemInterface. And then its normalize() method can delegate to the core serializer.

This would then allow us to remove $fallbackNormalizer as a concept from the JSON:API serializer entirely, as it would be encapsulated by FieldItemPropertyNormalizer.

Comments

effulgentsia created an issue. See original summary.

gabesullice’s picture

gabesullice
Primary language English
commented

Hmm, interesting idea! I like how clean it would be!

My concern is that this would be a massive a disruption for JSON:API Extras and by the time we would be able to do this without breaking extras, we'd be so close to getting rid of our custom serializer that we might as well just do that.

Maybe I'm not seeing something you are though.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +API-First Initiative

Definitely very interesting, and I like the explicitness.

Unfortunately, it's indeed practically impossible without breaking JSON:API Extra's FieldEnhancer concept, which allows developers to override/tweak the normalization at the @FieldType level instead of the @DataType level.
But … \Drupal\jsonapi_extras\Normalizer\FieldItemNormalizer::normalize() is already ignoring/not respecting @DataType-level normalizers like \Drupal\jsonapi\Normalizer\FieldItemNormalizer::normalize() is. Zero ::normalize() calls from inside the \Drupal\jsonapi_extras\Plugin\jsonapi\FieldEnhancer namespace. So … I actually don't see how JSON:API Extras would be impacted?

So can you explain this a bit more:

My concern is that this would be a massive a disruption for JSON:API Extras and by the time we would be able to do this without breaking extras, we'd be so close to getting rid of our custom serializer that we might as well just do that.

gabesullice’s picture

gabesullice
Primary language English
commented

So can you explain this a bit more

Yep. JSON:API Extras "overrides" entity normalizers by (ab)using the fallback behavior. To override/replace JSON:API normalizers, Extras removes JSON:API's normalizers and then adds its own normalizers to the core serializer. Thus, we force a "fall back" to the Extras normalizer. Extras then injects the JSON:API serializer into its normalizers so that it can re-enter the JSON:API serializer world.

Basically, Extras does some black magic with services and depends on the fallback behavior to make it possible.

I don't think there's a better way to replace that black magic without creating a public API, which we don't yet want to do. In one way, I see the complexity and "black magic" as a feature, not a bug (right now). There's just no possible way you can copy what Extras is doing without getting a deep feeling of "I shouldn't be doing this" lol and this is what I want.

effulgentsia’s picture

effulgentsia commented
Title: Restrict the fallback normalizer to field item properties only » [PP-1] Restrict the fallback normalizer to field item properties only
Status: Active » Postponed

Per #4, I'd like to postpone this on cleaning up Extras to not require the fallback backdoor. I haven't opened an issue for that yet, but I will soon.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

@effulgentsia I'm eagerly awaiting that issue :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

For now, I don't think there's anything we can do here, waiting for @effulgentsia.

xjm’s picture

xjm
she/her
Primary language English
commented

So to clarify, this is postponed because currently it'd be a BC break for the downstream contrib ecosystem?

effulgentsia’s picture

effulgentsia commented

Yes, and @gabesullice and I pair programmed today on the JSON:API Extras patch needed for it to stop relying on the fallback normalizer for its own things. He'll post that to the Extras queue.

gabesullice’s picture

gabesullice
Primary language English
commented
Title: [PP-1] Restrict the fallback normalizer to field item properties only » [PP-1] Remove the JSON:API serializer's fallback behavior and inject the core serializer in the field item normalizer
Related issues: +#3036904: Decorate JSON:API's normalizers and fake its namespace so JSON:API can eliminate its Serializer's fallback behavior.

Patch posted.

I think with that done, there's no need to have any fallback behavior, we can just inject the core serializer directly in the field item normalizer to be use directly. I think that's what the IS is saying already; I was confused because the title isn't accurate.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Remove the JSON:API serializer's fallback behavior and inject the core serializer in the field item normalizer » Remove the JSON:API serializer's fallback behavior and inject the core serializer in the field item normalizer
Status: Postponed » Active

#3036904: Decorate JSON:API's normalizers and fake its namespace so JSON:API can eliminate its Serializer's fallback behavior. landed.