Problem/Motivation

JSON:API Include currently flattens the entire structure of a JSON:API response as much as possible. In most cases, this results in the removal of the attributes and relationships key.

Under certain circumstances, Javascript based JSON deserializers may ignore flattened data since it is not accounted for or not 'to spec' with expected responses. JSONA, used by default by NextJS for Drupal, is one such case:

https://github.com/olosegres/jsona/blob/master/src/builders/JsonDeserial...

With attributes and relationships removed by JSON:API this data is removed from the end client (NextJS) and cannot be parsed.

Proposed resolution

Don't remove the attributes and relationship keys, but add the included result into their respective keys. This will result in not breaking downstream deserializers or require developers to write their own interpreter add on to account for that.

User interface changes

Potentially introduce configuration for users to decide if it should flatten the array as far as it can, or not.

API changes

Data model changes

Issue fork jsonapi_include-3460642

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kevinquillen created an issue. See original summary.

ptmkenny’s picture

ptmkenny commented
Category: Bug report » Feature request

ptmkenny opened merge request !21

ptmkenny’s picture

ptmkenny commented
Status: Active » Needs review

Thanks for this issue. I am now co-maintaining this module, and am attempting to clear out the issue queue.

It appears this module is designed to flatten the response, so I moved this to a feature request rather than a bug report.

I've added an MR that adds a new config option, "do not flatten", which will preserve the attribute and relationship keys. (Ideally, this would be a positive option like "Always flatten" rather than a negative one, but in the interest of preserving backwards compatibility, I am thinking that this is better as an opt-in checkbox.)

Please let me know what you think and if this MR works for your use case.

ptmkenny’s picture

ptmkenny commented
Version: 8.x-1.7 » 8.x-1.x-dev

alex bukach made their first commit to this issue’s fork.

ptmkenny’s picture

ptmkenny commented

ptmkenny credited jérôme dehorter.

ptmkenny’s picture

ptmkenny commented
mars0test’s picture

mars0test commented

Hi

I read the topic and don't really get why you are doing this.
Just to include by default relatioships linked I guess. It's weird to go in the exact opposite way of the module. It'es seams this module is doing a job on the Jsonapi module to astract the Hateoas approach, if you use it, it's to hide the complexicity of the model.

In the Hateoas model you can't force a consumer to get data that is not required (this is why I talk about abstract complexicity) Doing only half of the job or both of the job result to integrate something spectific to your needs.

If you use the model Hateoas use the react lib. But if you use Jsonapi:include module, use a standard model that match with hydra or classic json model.

jérôme dehorter changed the visibility of the branch allow_keeping_includes to hidden.

ptmkenny changed the visibility of the branch allow_keeping_includes to active.

ptmkenny’s picture

ptmkenny commented

@jérôme dehorter Please do not hide branches without leaving a comment about why you are doing so. I am the module maintainer and I wrote the MR, and I think it may be committed to the module, so the MR should stay open.

jérôme dehorter changed the visibility of the branch allow_keeping_includes to active.

jérôme dehorter’s picture

jérôme dehorter
Primary language French
Location Lille
commented

Hello @ptmkenny,

Sorry for this misclick. I change the visibility again.

kevinquillen’s picture

kevinquillen commented

#10 the reason for this is to not break compatibility with Javascript deserializers written for JSON:API specification. The module can still include everything in the relevant areas as long as it does not drop 'attributes' 'relationships' etc. I suspect many libraries like JSONA simply expect them to exist with values. Without them, it cannot parse responses. You can see this here:

https://github.com/olosegres/jsona/blob/master/src/builders/JsonDeserial...

here is another example in another library:

https://github.com/SeyZ/jsonapi-serializer/blob/master/lib/deserializer-...

or another:

https://github.com/ShadowManu/jsonapi-deserializer/blob/master/src/index...

This would require frontend developers to write their own parsers which can take several dozens of hours or more. At least with the option the structure can be preserved without losing the additional features (including all resources possible).

jérôme dehorter’s picture

jérôme dehorter
Primary language French
Location Lille
commented

Good morning,

I tested the allow_keeping_includes branch with the Do not minimize the response enabled configuration and I don't see any change.

When the cache is empty, I will see the JSONAPI response of my content and everything is well integrated. I refreshed the page and nothing is present.

Drupal 10.3.2
Next.js 2.0.0-beta1
Multiple JSON:API extensions

ptmkenny’s picture

ptmkenny commented
Status: Needs review » Needs work

"Needs work" as per #17.

lawxen’s picture

lawxen commented
Priority: Major » Critical

^1.8 and 8.x-1.x (with no any patch) both has the problem of #17, Did anyone knows how to quick fix it?

lawxen’s picture

lawxen commented

I find my problem in #18 is caused by the include in jsonapi_defaults（submodule of jsonapi_extra), The default include in jsonapi_defaults won't take effect on the second request after clear the cache

ptmkenny’s picture

ptmkenny commented
Priority: Critical » Major

@lawxen Please open a new issue if you found a bug in the dev branch; this is a feature request.

lawxen’s picture

lawxen commented

@ptmkenny New issue here Didn't support jsonapi_default

ptmkenny’s picture

ptmkenny commented
Status: Needs work » Needs review

@lawxen Thanks!

I'm setting this back to "Needs review" because the issue described in #17 and #20 is a separate bug that will be discussed here: #3490592: Incompatible with jsonapi_default

ptmkenny’s picture

ptmkenny commented
Version: 8.x-1.x-dev » 2.0.x-dev

New features now need to go into 2.0.x. 1.x will only receive security fixes.

ptmkenny’s picture

ptmkenny commented
Status: Needs review » Needs work

The MR needs to be rebased. I won't be working on this further, but I will commit the code if it gets rebased and tested.

divyansh.gupta’s picture

divyansh.gupta commented
Assigned: Unassigned » divyansh.gupta

Working on it!!

divyansh.gupta’s picture

divyansh.gupta commented
Assigned: divyansh.gupta » Unassigned
Status: Needs work » Needs review

I have successfully rebased the branch, so if somebody can review this!

ptmkenny’s picture

ptmkenny commented
Status: Needs review » Needs work

Thank you for rebasing, but the tests are now failing.

lieb’s picture

lieb commented

Drupal Canvas uses the Jsona serializer with JsonApiClient. I have confirmed that if I have jsonapi_include running on the source site, Canvas is not able to consume content via JsonApiClient. I have jsonapi_include 2.0 on my source site. I would be happy to test a dev version if there is one that addresses this issue. Thank you.

ptmkenny’s picture

ptmkenny commented

@lieb You can test the version on the branch allow_keeping_includes. Tests are broken and the types are not correct, so this will not be committed as is, but you can see if that approach works for you.