Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
We've had to update the EntityToJsonApi service many times, both to fix flaws in the code and to keep in sync with upstream.
Proposed resolution
I've been wanting to change it to use subrequests for a while to solve this problem once and for all, since we then just rely on the upstream JSON:API module.
Last week, @e0ipso indicated he now agreed that it'd be better.
So let's do it!
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3035045-8.patch | 8.08 KB | Wim Leers |
| #8 | interdiff.txt | 1.09 KB | Wim Leers |
| #4 | 3035045-4.patch | 7.96 KB | Wim Leers |
| |||
Comments
Comment #2
Wim LeersThis just came up again in #3032679-24: Clean-up: rename EntityCollection to Data. Rather than once again updating this service, this issue would remove the need for that change :)
Comment #3
e0ipsoI'm +1. I don't love the pattern, but it solves a real problem with maintenance.
Comment #4
Wim LeersComment #5
e0ipsoI think we should construct the URL based on the route name. That's because the actual path may vary based on the JSON:API Extras configuration.
Additionally, would it make sense to have the subrequest processed as MASTER request? That way we can prime the page cache and leverage page cache.
Comment #6
e0ipsoComment #7
Wim LeersI wanted to avoid coupling it to route name internal details, but I forgot about this. Great point! Will do.
Comment #8
Wim LeersComment #10
e0ipsoMerged!
Comment #11
Wim Leers🙏
Comment #12
sphism CreditAttribution: sphism commented@e0ipso just to let you know that a fresh install of Contenta hits this issue as soon as you view your content, it's not a good look :)
Comment #13
e0ipso@sphism thanks for bringing it up. I know it is frustrating to update your project in the worst possible moment. Please be a bit more patient while the fixes trickle in.
Comment #14
sphism CreditAttribution: sphism commented@e0ipso no worries, I was just evaluating Contenta for a new project and as it stands right now this is a critical blocker from me using it, i.e. viewing any content returns nasty error... I tried to fix it but d8 is all pretty new to me and I couldn't get it to work at all.
Comment #15
e0ipsoThis should be available to Contenta CMS and other distributions. 🎉
Comment #16
Wim Leers🥳
Comment #17
sphism CreditAttribution: sphism commentedawesome, i'll try out contenta again... although it looks like we'll be using straight d8 now
Comment #18
Wim Leers#17: 🥳
Comment #19
miiimoooI think it breaks now *because* it is done as a subrequest - see #3044925: Upgrade broke normalize in preprocess: LogicException: Render context is empty, because render() was called outside of a renderRoot() or renderPlain() call
Is there any point in reverting this? Either approach has complexity, but the subrequest involves the whole symfony request processing so it's difficult to diagnose or fix this there
Apologies, if this is not the place to report this.