Sorry in advance for the dump of info on where I'm up to on some debugging, I'm not sure where to propose a fix for this:
I've hit a bug in a project I'm working on where a client-side navigation is causing the page that is being navigated to to crash. The page has a listing and the listing block relies on EffectiveDate being available to display correctly. The listing blocks get their listingItems prop from the withQuerystringResults HOC. When the client-side routing happens, there doesn't seem to be any subrequests in the querystringsearch redux store yet, and so it falls back to the content.items list which doesn't include all properties of the children. This explains the crash. However, this same problem doesn't occur if I visit the URL of the page directly because page is SSR'd and so the subrequest ID of the listing block can be inserted ready for the page. As the subrequest id is kept in the redux store, navigating away and back to the page through client transitions doesn't cause a breakage as the ID still exists.
Is this a bug in the withQuerystringResults HOC? My thoughts are that the useDeepCompareEffect hook is run too late on the client for it to call getQueryStringResults, but I'm not sure if there's other things at play here
I'm not a fan of the duality of the listing block as "folder contents" and "plone collection". It leads to complicated code in that HOC and plenty of other edge cases.
On the other question:
useDeepCompareEffecthook is run too late on the client for it to call getQueryStringResults
No, I don't think you should go in this direction. Just consider a simple React listing component which needs to fetch remote data. It should do that in an async, controlled way, right? So we have to do it in an useEffect. If it's a "use effect", the first render of that component will need to take into account that the items haven't been fetched yet, so it needs to deal with items = []. Only after the async data has arrived, will it have the full list of items. Btw, useDeepCompareEffect is just like useEffect, with the only difference in that it does a deep equality check for the dependency list (in our case, adaptedQuery is a complex object, so it needs to be deep checked).
@JeffersonBledsoe I also ran into this same problem. I think you are having the problem of not using a search criteria in the listing block. See comments at:
See too:
Briefly, what happens is this:
When using the listing block without a search criteria, withQuerystringResults uses getContent, which does not return all metadata:
When using the listing block with a search criteria, withQuerystringResults uses getQueryStringResults, which contains all the metadata:
The SSR request uses getAsyncData.js, which uses getQueryStringResults and returns all metadata:
I proposed the fix:
Which uses getQueryStringResults and always returns all metadata in both situations (with and without search criteria). But it still seems to be up for debate.
On second thought, well, maybe the default shouldn't return all the metadata, for performance reasons, since the default variations work just as well as the basic metadata. It would be the variations/customizations that should inform which metadata they need.
It's a bug, but it's subtle. The && querystringResults?.[block] bit should not be there and your problem would be fixed. Please submit a PR for this.
@tiberiuichim Ahh, force the listing to be fetched client-side by rendering an empty list initially until the subrequest has been updated and completed? This would solve the issue, but wouldn't I get a flash of an empty list before? I've been experimenting with remix.run the last couple of weeks and having a clear boundary of what is client and what is server-side has been a really nice DX.
@wesleybl Thanks for the suggestion! This wasn't the case this time, but we've definitely ran into this problem multiple times before as well. I like the PR and also agree that it would be nice to be able to customise which metadata is needed. Maybe having all metadata as the default with the ability to then specify a filter would lead to avoiding these bugs whilst still offering performance improvements? I believe the ObjectListWidget does something similar with being able to specify what information will be available.
The "flash of no content" happens all the time, this is not a problem unique to Volto. The typical solution is to show a loader, or a placeholder text, such as "Loading content..." or whatever.
I can't comment on remix.run, but in many ways Volto is its own class of application, where the "normal solutions" can't really apply. For example, file-based routing, we can't do that with Volto, as all our content is dynamic and defined in a database. Nevermind static site generation, it can be done but to implement it "at scale", when backed by a database such as Plone's, it's not easy. Lazy-loading or module federation and all the other static-introspection based tricks that the new frameworks do, they're based on static route definitions or other form of tight control over the entire code base. We can't do that as well, a Volto page can define and load any number of blocks, coming from different addons. And so on.
To me, the "next big thing" that would benefit Volto the most would be React's server components, as it would allow shipping less JS code to the browsers. But they're really new, there would be a lot of "upfront cost" to do them now (in terms of Volto community developers time), we can wait for a bit more, to see if we get "more for free" from the rest of the React community.