davnicwil / react-frontload

Async data loading for React components, on client & server
451 stars 21 forks source link

Nested frontloaded components #18

Closed matthewlein closed 5 years ago

matthewlein commented 5 years ago

Hello,

I really love how frontload works, its been great for me so far. The scenario of nested frontloaded components that are rendered on the second render has come up. I imagine the server side renderer would need to keep running render until all new frontload functions are resolved instead of just twice.

For example

given url /categories/[seo-category-name] First render:

Root ->  get all categories to map seo names to api friendly ids `api/categories`

Second render renders a new frontloaded component based on first render data

Root
  CategoryPage -> get category data `api/categories/[category-id]`

From what I can see, the second render does not call frontload again, so the nested components do not fetch data on a server side render. On normal navigation, it works fine because everything runs on mount.

Curious if you have any thoughts on how this could work? (without changing the API to accommodate data fetching strategies)

davnicwil commented 5 years ago

Hi @matthewlein thanks for the kind words and really glad it's useful for you.

Sorry for late reply here I've just returned from holiday!

So, yeah, this is something that I've seen before and have thought about a bit. The bottom line is you are right, frontload doesn't support nesting on server render because of the fixed 2 render passes design.

So far I've dealt with this simply by 'flattening' all frontload components to the top level.

On the one hand this has benefits, I think, in simplification of design and code maintainability as in a lot of cases flat is just simpler than nested, and fits in quite well with the Container / Presentational components pattern.

But on the other hand, if you take this too far it starts to remove the purported benefits of frontload, which is that data loading can be extremely localised to individual components. It's still a lot better than, say, loading all the data required for everything in the page at the route level, but it doesn't truly solve the problem, just divides it up a bit.

One possible solution I think would be to do the renders recursively, until the output is equal, then return. I haven't fully thought this through and there could be edge cases where it would not work, but I think it should. It would look something like this (pseudocode)

const frontloadServerRender = async (render, lastRenderedHtml, passNumber = 1) => {
  let nextRenderedHtml = await render()

  if (lastRenderedHtml !== nextRenderedHtml) {
    console.log(`render pass  ${passNumber} - not finished, doing another pass`)
    return frontloadServerRender(render, nextRenderedHtml, passNumber + 1)
  }

  console.log(`render pass  ${passNumber} - finished, returning markup`)
  return nextRenderHtml
}
matthewlein commented 5 years ago

Awesome, I hope you enjoyed some time off, its a very important part of the biz...

Thanks for taking the time to think about it. I have also been working around it by moving data fetching up the tree to higher levels, and for the most part it works really well.

That's an interesting approach. I could also imagine some edge cases where that could get in a loop, but you could always add a max depth where it returns regardless. But seems like a solid option to explore. I will let you know if I have something working.

davnicwil commented 5 years ago

Thanks, was a great holiday yes!

I think a max depth would make a lot of sense. Nested components firing chains of promises in serial are going to mean, in almost all cases, longer wait times before all content is loaded. This is less of an issue on the client where loaders can be shown whilst each progressive layer is rendered, but on the server it would certainly be more noticeable having to wait until the very last layer is loaded before returning.

A max depth limit could also help with this -- just telling the server to stop after a certain number of passes for general performance reasons then just render the rest on the client - the tricky thing here though is how to get those frontloads nested past the limit to 'know' they need to run on the first client render?

davnicwil commented 5 years ago

@matthewlein I've implemented this feature in #20

If you have time I'd appreciate if you'd give feedback and thoughts on whether I've done it the way you were expecting and if the documentation makes sense. Decided to go with the limit rather than just turning it on/off with a boolean for the reason you suggested above - to avoid infinite loops or just accidentally very deep nesting in unexpected corner cases etc, which would effectively just break server render.

It's been a bit of a challenge to document succinctly but thoroughly because there is so much nuance to it and so much of if/how to use this feature will depend on the application. But I've given it my best first shot!