nuxt icon indicating copy to clipboard operation
nuxt copied to clipboard

Multiple `useLazyAsyncData` within a composable becomes non lazy

Open huang-julien opened this issue 2 years ago • 7 comments

Environment


  • Operating System: Windows_NT
  • Node Version: v20.2.0
  • Nuxt Version: 3.7.4
  • CLI Version: 3.10.0
  • Nitro Version: 2.8.1
  • Package Manager: [email protected]
  • Builder: -
  • User Config: -
  • Runtime Modules: -
  • Build Modules: -

Reproduction

https://stackblitz.com/edit/nuxt-starter-t7zx5g?file=composables%2Ftest.ts

  • click on to lazy
  • the navigation is blocked

Describe the bug

Using 2 useLazyAsyncData make the page change blocking

Additional context

No response

Logs

No response

huang-julien avatar Jan 03 '24 14:01 huang-julien

it looks like getCurrentInstance doesn't exist for second useLazyAsyncData. So instead of going here https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/app/composables/asyncData.ts#L359-L362 it goes to https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/app/composables/asyncData.ts#L363-L365

gibkigonzo avatar Feb 03 '24 14:02 gibkigonzo

I believe that this is working correctly and that the issue lies with how it is documented. The example you provided @huang-julien uses await on useLazyAsyncData but these composables are not async and I think the awaits are causing the problem.

I believe the awaits make the vue component instance become undefined for any sequential await calls (ref: https://nuxt.com/docs/api/composables/use-nuxt-app ). And since instance is undefined, lazyAsyncData has to revert to just asyncData which blocks the route change because it can't call instance._nuxtOnBeforeMountCbs.push(initialFetch). This is also why one useLazyAsyncData with await works fine because the component instance is there on the first call.

Playground with working multiple useLazyAsyncData: https://stackblitz.com/edit/nuxt-starter-3ojz7s?file=pages%2Findex.vue

If this is correct, I suggest we make a simple doc change to remove the superfluous await. https://nuxt.com/docs/api/composables/use-lazy-async-data

jamesdawsonWD avatar Feb 18 '24 17:02 jamesdawsonWD

Didn't had the time to investigate yet, but it can be a unctx issue

huang-julien avatar Feb 19 '24 08:02 huang-julien

Ah, if removing await can cause a vulnerability then I will continue investigating. I am very new to this codebase, but it seems like nuxtApp.runWithContext isn't ensuring that vue component instance is available - but maybe that is not it's function? I will look into ways to ensure that the component ctx is available

jamesdawsonWD avatar Feb 19 '24 09:02 jamesdawsonWD

Ah, if removing await can cause a vulnerability then I will continue investigating. I am very new to this codebase, but it seems like nuxtApp.runWithContext isn't ensuring that vue component instance is available - but maybe that is not it's function? I will look into ways to ensure that the component ctx is available

The issue is actually even one step before that. The entire nuxtApp is undefined in this case. As the docs state: async context only works with <script setup>, defineNuxtPlugin and defineNuxtRouteMiddleware but not in composables.

Text from Docs

Vue currently only supports async context restoration for

General Advice

The pattern of awaiting in composables or in runtimeHooks (e.g. onMounted) should be avoided. After the first instance of await the nuxtApp will no longer be available and all composables that rely on nuxtApp break (even framework-internal composables like useAsyncData).

Bigger Issue

Using the described pattern not only breaks the lazy behaviour but potentially show a 500 internal server error to your users. This happens specifically when you don't use client-navigation to open the page using your composable (e.g. via the button in the reproduction), but rather enter the url to that page directly.

Workarounds

Workaround 1

You can put the await useLazyAsyncData directly in your <script setup>. Demo1

I'd still recommend removing the await keyword so the requests run in parallel

Workaround 2

If you want / need the code to be inside a composable and it needs to be awaited, you can wrap the calls (Without awaiting each individual call or your site breaks again!) in a await Promis.allSettled and access the results. This is a bit more verbose, but it's closer to your original reproduction. Demo2

felix-dolderer avatar Aug 27 '24 14:08 felix-dolderer

Given my answer above, I still don't fully understand what's going on in depth. When playing around with this I noticed the first await-call behaves differently from the following (similar to this issue #28562 ).

It's not only that the nuxtApp is only available in the first pass (until the first awaited call), but also I don't understand the order of execution.

Say I have the following code:

// ignore incorrect syntax and look at reproduction
onMounted(async () => {
  await useLazyAsyncData(1)
  await useLazyAsyncData(2)
  await useLazyAsyncData(3)
  await useLazyAsyncData(4)
})

/*
Execution seems to be in order, except the first call comes last:
2 -> 3 -> 4 -> 1
*/

Reproduction https://stackblitz.com/edit/nuxt-starter-xdxexc?file=composables%2FdemoLazyAsyncBroken.ts Hint: Open the browser logs and navigate to /demo3 using the button on the page to observe the described behaviour.

Why is the first call (the only one that has the nuxtApp instance available) executed only after all the other "faulty" calls are done? I can't find the code responsible for this.

felix-dolderer avatar Aug 27 '24 14:08 felix-dolderer