Sequence fetch/asyncData for nested route pages

bjunc
114
bjunc
commented 2 years ago

In a nested parent / child page relationship (using <nuxt-child />), it would be ideal if the parent's fetch/asyncData functions ran before the children. Currently, they all seem to fire at once (even if async/await is used). Please correct me if I am wrong here.

For example, in the routing docs, we'd want category to complete its fetch before subCategory ran its fetch. That would make the parent category data available to the subCategory (eg. via the store).

Without this sequence, if you deep link to the subCategory, the parent category data may not have been set yet; preventing route checks that depend on that parent data, and/or the ability to append more granular data (unless dummy placeholders are added to the store). This essentially requires the subCategory to load the category data as part of the fetch; which results in the same data being loaded twice (once for the category and once for the subCategory).

Thoughts?

1
arenddeboer
30
arenddeboer
commented 2 years ago

I looked into this myself because I wanted to be able to redirect from a child component by returning a response object from the asyncData function with a redirect url property. The problem I think lies in this piece of code:
https://github.com/nuxt/nuxt.js/blob/ca13b6bfbbce8992046b5dc5294f104c40950bb2/lib/app/client.js

let promises = []
     const hasAsyncData = Component.options.asyncData && typeof Component.options.asyncData === 'function'
     const hasFetch = !!Component.options.fetch
     <% if(loading) { %>const loadingIncrease = (hasAsyncData && hasFetch) ? 30 : 45<% } %>
     // Call asyncData(context)
     if (hasAsyncData) {
       const promise = promisify(Component.options.asyncData, app.context)
       .then(asyncDataResult => {
         applyAsyncData(Component, asyncDataResult)
         <% if(loading) { %>if(this.$loading.increase) this.$loading.increase(loadingIncrease)<% } %>
       })
       promises.push(promise)
     }
     // Call fetch(context)
     if (hasFetch) {
       let p = Component.options.fetch(app.context)
       if (!p || (!(p instanceof Promise) && (typeof p.then !== 'function'))) {
           p = Promise.resolve(p)
       }
       p.then(fetchResult => {
         <% if(loading) { %>if(this.$loading.increase) this.$loading.increase(loadingIncrease)<% } %>
       })
       promises.push(p)
     }
     return Promise.all(promises)

As you can see, this executes all promises and waits until all of them are finished. To do this in sequence you probably need to modify this core file. I was able to use Bluebirds Promise.each on a different project. But this project uses a slightly different approach as you can see here: https://github.com/vuejs/vue-hackernews-2.0/blob/master/src/entry-client.js
Anyway, I could be off here, but it might give you some pointers.

0
ndamjan
0
ndamjan
commented 2 years ago

I was able to adjust the promise logic in the core files (client.js and server.js) found in node_modules/nuxt/lib/app which seems responsible for this, so far I couldn't see any side-effects.

Attached are the two files if you'd like to test it as well.

Archive.zip

Maybe someone from the core team can evaluate if this impacts some other part of the framework?

0
qm3ster
383
qm3ster
commented 2 years ago

With this modification, how can we still run them in parallel?

0
ndamjan
0
ndamjan
commented 2 years ago

Perhaps this sequential loading could be an opt-in option in nuxt.config in order to preserve the current behaviour. So far I have done this only for myself, once there is some feedback about this approach I can do a pull request for this feature.

0
qm3ster
383
qm3ster
commented 2 years ago

How about you pass the promise of the mother page into the daughter page's method instead?
Then you can await it if you need, and ignore it if you don't.
Seems over 9 gorillion % more flexible than enabling it in global config.

You could even start some independent async shenanigans first, then await and do the rest.

0
bjunc
114
bjunc
commented 2 years ago

I think of it somewhat how async.js does, in that there are multiple options to choose from. What I am proposing, would be described as "series":

Run the functions in the tasks collection in series, each one running once the previous function has completed. If any functions in the series pass an error to its callback, no more functions are run, and callback is immediately called with the value of the error. Otherwise, callback receives an array of results when tasks have completed.

Maybe Nuxt could one day support many options, but having "series" and "parallel" (default) would be a great start.

Per config, it'd be great if this could be set at the root page level. You may not want the same behavior throughout the app. If we borrow the async.js vocabulary, then we could add a "controlFlow" property on the parent page. Example:

export default {
  controlFlow: 'series',
  fetch (context) {
    ...
  }
}
0
bjunc
114
bjunc
commented 2 years ago

@qm3ster in async.js terminology, I believe that would be the "compose" option.

Creates a function which is a composition of the passed asynchronous functions. Each function consumes the return value of the function that follows. Composing functions f(), g(), and h() would produce the result of f(g(h())), only this version uses callbacks to obtain the return values.

So, in our case, we'd reverse the order so that parentPageFetch(), childPageFetch(), and grandchildPageFetch() become grandchildPageFetch(childPageFetch(parentPageFetch()))

I think this may not be necessary though, as you could accomplish the same thing by assigning to the store upon each fetch in series.

0
qm3ster
383
qm3ster
commented 2 years ago

@bjunc pretty much, except unlike in the callback case, it would be almost literally childPageFetch(parentPageFetch()), wherein parentPageFetch() returns a Promise.
This way, every child decides when and if it cares about the parent Page's completion.
It can even depend on their params.

0
bjunc
114
bjunc
commented 2 years ago

@qm3ster Works for me. Is the idea that we'd assign the parent's promise to context as to not break backwards compatibility? Any proposed attr names? context.parentFetchPromise and context.parentAsyncDataPromise?

0
qm3ster
383
qm3ster
commented 2 years ago

context is shared between the child and the parent IIRC, so that's way too globalicious.
I was thinking a second argument.
Whether that should be a stack of promises all the way up or just the immediate parent is a good question. (what do we do in the possibly very rare case of a grandchild wanting to wait for the top page?)

0
bjunc
114
bjunc
commented 2 years ago

Smells a bit over-engineered. If you run the page fetches in series, then the grandchild will have access to the parent and grandparent's data (assuming you assign to the store upon responses).

0
qm3ster
383
qm3ster
commented 2 years ago

@bjunc if the child wants the grandparent's data, but the paren't returned without awaiting the grandparent, the child has nothing to await.

0
qm3ster
383
qm3ster
commented 2 years ago

EASILY solved by the user by awaiting the grandparent at the end of the parent. But still more to write (for the user) than in my case.
I guess I'll try implementing my way.

0
qm3ster
383
qm3ster
commented 2 years ago

HILARIOUS idea: what if we also give fetch and asyncData each other's promises?
That way they can rely on each other.
And bad users can then cause deadlocks by mutually awaiting them both!
Amazing, if I do say so myself!

0
bjunc
114
bjunc
commented 2 years ago

@ndamjan the logic you posted worked for me. To @qm3ster's point, it replaces the parallel logic, so we'd need a switch for parallel vs series. I was able use an option on the root page component to trigger series vs parallel (used controlFlow: 'series' as per above). It's basically an if statement, so let asyncDatas needs to be set outside to allow for access in the later debug and ssrContext.nuxt.data = asyncDatas.map logic.

I think the logic could be cleaned up a bit, but overall heading in the direction I was hoping for. Maybe use reduce instead of forEach?

0
qm3ster
383
qm3ster
commented 2 years ago

I don't understand the advantage of this method. It just makes it too easy to damage performance, in addition to being inflexible. Promises are simple to add, we already have the source of the promises in the code.

0
bjunc
114
bjunc
commented 2 years ago

I think that's a dramatized critique. The phrase "damage to performance" implies unintentional consequence, when in-fact it's by design in order to accomplish the functional goal. The whole point is to wait for the parent to load its data before the child starts loading theirs. The worst performance is what we have now – as I can't wait for the parent to finish, so I have to redundantly load the data twice (once in the parent, and once in the child).

The "inflexibility" part doesn't make sense to me either, as you're not proposing a scenario that neither the current (parallel) or new (series) logic would accommodate.

All that said, I'll try your technique now that the first one has been prototyped and deemed feasible. It's not immediately clear how the promises should be keyed, but I'll think that through. Maybe their component names, but those can get pretty long. Anyway, I'll take a stab at it, and report back.

0
bjunc
114
bjunc
commented 2 years ago

Okay, so I hacked together the alternate method (Option B). I added an app.context.fetchStack array. The array consists of objects that use the following structure:

[
  {
    component: 'pages/grandparents/_grandparentId.vue',
    asyncData: null,
    fetch: null
  },
  {
    component: 'pages/grandparents/_grandparentId/parents/_parentId.vue',
    asyncData: null,
    fetch: null
  },
  {
    component: 'pages/grandparents/_grandparentId/parents/_parentId/children/_childId.vue',
    asyncData: null,
    fetch: null
  },
]

To await an ancestor's fetch method:

// _childId.vue
export default {
  fetch ({ fetchStack, params }) {
    let parent = fetchStack.find(x => x.component === 'pages/grandparents/_grandparentId/parents/_parentId.vue')
    await parent.fetch

    // Check if parent has children matching `params.childId`
    // No: error 404
    // Yes: fetch full child data
  }
}

Option A (setting series vs parallel for the whole stack) is cleaner in that you don't need to explicitly await in a child page. My guess is that more often than not, where you need to await an ancestor, you probably want to await all ancestors.

Option B is potentially more performant in situations where an ancestor's page is not needed, so you can be more granular / selective with your series vs parallel in exchange for being more explicit in your code.

@qm3ster I think I know which option you prefer :)
Anyone else want to weigh in before I start hardening the logic?

0
ndamjan
0
ndamjan
commented 2 years ago

@bjunc I was thinking about the same structure :)

My guess is that more often than not, where you need to await an ancestor, you probably want to await all ancestors.

To support above statement which I agree with as well, I propose we make it easier to do the series use case by having all of the ancestor promises also delivered additionally as a single promise.

In this case, the fetchStack would have an object with details (the structure from previous post which you can use to await on a specific grandparent) and another ancestorDataFetch promise which contains all of the promises from all ancestors.

Then in the child component we can simply do

fetch ({ fetchStack, params }) {
  await fetchStack.ancestorDataFetch
  // now all data is here... 
}

We will need to implement this in all of the ancestors in order to get the series effect, but this kind of approach makes it explicit.

0
qm3ster
383
qm3ster
commented 2 years ago

And what happens when you navigate to the context.fetchStack when you navigate to the next page? It's set to an empty array, then populated by the parts of the new page?
What if you navigate only the child page?

0
bjunc
114
bjunc
commented 2 years ago

I'm not sure you'd need that. Since you have access to an array of promises, you can use a combination of array methods and Promise.all() to accomplish different goals. For instance, if you wanted to wait till all ancestor fetches completed:

{
  fetch ({ fetchStack }) {
    // await all ancestors' fetch promises
    const fetchStackIndex = fetchStack.findIndex(x => x.component === this.name)
    await Promise.all(fetchStack.map((x, index) => index < fetchStackIndex ? x.fetch : null))

    // now you can safely fetch knowing the ancestor data has been loaded
  }
}

That's not "series" per-se, but rather a group of parallel ancestor promises. If you needed each page component to await it's ancestors, then you'd need to add that logic on every page component in the tree; which is why I was saying that I think that's a bit verbose for a scenario that is more likely to happen than cherry-picking. However, I suppose it can be DRY'd up.

0
bjunc
114
bjunc
commented 2 years ago

@qm3ster I've been isolating this to process.server so far (start simple, demonstrate feasibility, then add layers of complexity). When navigating between sibling pages the ancestor pages are already loaded, so I'm not sure that's an issue (unconfirmed). It would however be an issue when "deep linking" across the site (new ancestor tree). I'll look into it, but my initial thought is that the stack would be replaced with a new array as if this were a server request (also unconfirmed).

0
qm3ster
383
qm3ster
commented 2 years ago

the stack would be replaced with a new array as if this were a server request

With the top pages already resolved?

0
bjunc
114
bjunc
commented 2 years ago

I'm not an expert on Promises, so I'd have to confirm this, but my understanding is that if you reference an already resolved Promise, then the value is returned, and any calls to Promise.all() would behave as expected. So you could replace the entire stack, and any child await calls would simply progress instantly for the ancestors that were already loaded from previous routing. Can you confirm/deny this?

I can investigate a little later. Have to get some actual work done :)

0
qm3ster
383
qm3ster
commented 2 years ago

That's exactly how they work, yes. I meant that the ancestor's promises would be gone when you are replacing the stack, so new children wouldn't receive them.

0
iddogino
0
iddogino
commented a year ago

Has anyone found a good solution for this that doesn't require core modifications (to be used now until this is implemented)?

0
cliffrowley
0
cliffrowley
commented a year ago

This would be a really great feature to have as currently it makes working with rootState problematic in SSR, since the store mutations are called after the child's asyncData and fetch are called. With this implemented, it would make the store a lot more useful (and tidy).

0
ndamjan
0
ndamjan
commented a year ago

@iddogino in the end I implemented an additional request queuing mechanism in the store. When I need to respect the order of API calls in my project, instead of using axios within fetch function I call await dispatch('request/get', { .... This puts the parameters in an internal queue and executes requests one by one.

0
bjunc
114
bjunc
commented a year ago

I ended up having to mothball this to focus on other things. I'm still very interested in it though, and there are many places through our app that could greatly decrease initial server-rendered page load time by loading page data in series.

I think ultimately the best way to accomplish this is to modify the core to provide access to the page fetch stack promises, but I have admittedly fallen behind in keeping up with Nuxt edge releases and not sure if a pull request would conflict with (or be applicable to) Nuxt 2.0. I'd have to really block out time for this; which I'm unfortunately not able to do right now. Hopefully in a month or so I can really dig in.

0
Informations
QuestionUnresolved
#c2457 - Created 2 years ago