cms
cms copied to clipboard
statamic
Handle query builder when using json_encode or to_json.
This fixes #5866, #6130 and other related to to_json bugs. (also tested in Duncan's demo of this bug)
Since 3.3 we have a lot of trouble with the to_jsonmodifer or with custom PHP code that uses json_encode when trying to pass data to Vue. Generally, when using the fields that now return QueryBuilder instances like Assets or Taxonomy Terms nested inside Bard or Replicator fields, when converting them to JSON they turn out empty.
I am not sure if this is a complete (or even correct) fix, I am just hoping to speed things up as we have to keep many sites at 3.2 because fixing this issue otherwise requires changing a lot of frontend code.
Thanks!
I would love some help here. I've checked the toJson method in the core modifer and tried to combine these but I'm not sure it would work in all cases. I've also tried using the isQueryBuilder method from the Comparator class but it doesn't seem to catch this.
Using the first commit of this PR I noticed severe slowness (it would timeout at 30secs when getting ~30 entries). We tried to add ->toAugmentedArray() which fixed the slowness but that threw a JSON depth error in some cases. We ended up changing the order and it now seems to work.
I can confirm it also fixes Duncan's repository but I'm not sure how to properly test this. For now we are using it in production using composer-patches.
The PR in its current state is fine. It'll be slow because there will essentially be n+1's happening.
I noticed you tried to fix it and then reverted it all.
If you want to speed it up, you'll need to do more detailed querying rather than just getting the top level stuff and relying on json encoding the whole thing.
Could you provide a repo that shows a more realistic view of what you're trying to do? We could help work out what to suggest then.
I swapped the order of the added code in b33cc81 and it seems to have improved the speed a lot.
I think Duncan's repo pretty much sums it up. We use Vue extensively in some websites, getting the whole collection and then using {items | to_json | entities} to pass everything to Vue to do filtering, templating, pagination there. Without this PR, Assets, Entries and Taxonomy Terms come out empty (well you can see it in the related bugs I guess).
If you feel this fix is adequate I guess you can merge this. It fixed the issue in our cases for sure.
Oh I see. I misread the commits. I thought you reverted it, but you actually moved it earlier. 👍
@jasonvarga any reason why this isn't getting merged? We have to composer-patch quite a few websites in order to move to 3.3. :)
It might fix your particular use case but it's not enough of a general fix. I was working on it locally the other day but had to stop for something else.
OK fair enough! Thanks!
Please allow me to add that it's not really a weird use case, it's how to docs suggest passing data to frontend frameworks: https://statamic.dev/javascript-frameworks I'm afraid more people are having issues with this and end up wasting time trying to debug it elsewhere.
Hey @jasonvarga this still bothers us every now and then so I thought I'd bother you in turn. :)
Hey there 🙂
Unfortunately we are experiencing the same issue in v4.17.0. As @afonic suggests, is a use case suggested in the Docs. We want to use page data on Vue/React but entry-based fields come empty...
- Can someone from Statamic team give us a hint if they're planning to test and merge this anytime soon, @jasonvarga?
- Is it possible to apply the fix suggested on this PR to our build, @afonic? I'm really interested on testing this.
Thanks to everyone!
The team will get to the PR when they're able to.
In the meantime, you can use a composer patch to bring this PR into your project.
The team will get to the PR when they're able to.
In the meantime, you can use a composer patch to bring this PR into your project.
I really appreciate your answer, @duncanmcclean 🙏🏽 Thanks a lot