Skip to content

feat(menu): support deferred lifecycle-safe loading - #461

Open
StirStudios wants to merge 1 commit into
drunomics:2.xfrom
StirStudios:fix/fetchmenu-hydration
Open

feat(menu): support deferred lifecycle-safe loading#461
StirStudios wants to merge 1 commit into
drunomics:2.xfrom
StirStudios:fix/fetchmenu-hydration

Conversation

@StirStudios

@StirStudios StirStudios commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • rebuild this branch on the latest upstream 2.x
  • add a new useMenu() composable that returns Nuxt AsyncData
  • keep fetchMenu() backward compatible for setup-time use
  • document deferred, client-only menu loading
  • add regression coverage for deferred execution

Underlying issue

fetchMenu() wraps Nuxt's useFetch(). When it is first called from onMounted() during hydration, Nuxt registers the immediate request for the component's beforeMount lifecycle. That lifecycle has already completed, so the request never executes and the menu data remains undefined until another navigation or render cycle occurs.

A direct client-side $fetch fallback avoided the symptom, but bypassed the normal AsyncData contract and did not address the lifecycle mismatch.

Proposed solution

useMenu() creates the AsyncData request during setup and exposes execute(). Consumers that need lifecycle-controlled loading can initialize it with immediate: false and call execute() later:

const { useMenu } = useDrupalCe()

const { data: accountMenu, execute: loadAccountMenu } = useMenu('account', {
  immediate: false,
  server: false,
})

onMounted(() => loadAccountMenu())

Existing setup-time callers can continue using fetchMenu() unchanged.

Validation

  • Nuxt and packaging suites: 19 files, 103 tests passed
  • focused menu suite: 7 tests passed
  • changed-file ESLint passed
  • package build/prepack passed

The full local E2E command could not complete because the test ports were already occupied and the expected Playwright browser binary was not installed; GitHub CI is running the complete project workflow.

@fago

fago commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

sry for letting this lie so long. @StirStudios Could you clarify why this is needed? usually the menu-fetching should work just fine client-side as well + hydrate properly also?

@StirStudios

Copy link
Copy Markdown
Contributor Author

This fails for us on a full page refresh when used in the admin menu — specifically within the user account menu.

Without this in place, the user menu breaks on full page refreshes.

@fago

fago commented May 6, 2026

Copy link
Copy Markdown
Contributor

hm, weird.

It seems to me that this special-casing is not the proper fix, it adds a work-around to the regular code-path, but why?

Could you clarify why the regular fetch is failing in your case? Maybe I'm missing the obvious, but what's wrong with it? How/why does it fail?

@StirStudios

Copy link
Copy Markdown
Contributor Author

Thanks @fago — I think you’re right that the current patch is more of a workaround than the ideal fix.

After looking closer, I don’t think the issue is simply “client fetch after hydration.” The more likely issue is that the account menu is session/user-dependent, but fetchMenu() currently defaults to a generic cache key:

useFetchOptions.key = useFetchOptions.key || `menu-${name}`

So for the account menu, the SSR result can be cached as:

menu-account

Then after a full browser refresh, hydration reuses that cached payload instead of fetching the account menu again with the active browser session.

That explains the behavior we see:

  • Full refresh: account/user menu does not populate correctly
  • Soft Nuxt navigation: account/user menu works
  • Direct client-side $fetch after mount: works
  • Drupal endpoint itself works
  • The failure is specific to the SSR payload/hydration path for a session-dependent menu

So I agree the better fix is probably not a broad client-side $fetch fallback. A better direction may be to make fetchMenu() avoid reusing a generic payload cache for session-dependent menus, or allow callers to provide a user/session-specific key for menus like account.

For example, instead of always defaulting to:

menu-${name}

we could support safer behavior for user-dependent menus via an explicit key:

await fetchMenu('account', {
  key: `menu-account-${currentUserId}`
})

Or document that session-dependent menus must provide a cache key that varies by user/session.

Another possible fix would be to avoid payload reuse for menus that are known to depend on the active session, but that may be too implicit.

So I’m happy to revise this PR away from the hydration $fetch fallback and toward fixing the actual cache/key behavior in fetchMenu().

@StirStudios

Copy link
Copy Markdown
Contributor Author

Update after deeper testing on our side:

I reverted to a clean baseline and re-tested with local package builds in a real app integration (hard refresh + soft navigation),
and the key/cache-only proposal is not sufficient to resolve the issue end-to-end.

What we confirmed:

  • Soft Nuxt navigation continues to populate account menu.
  • Hard browser refresh still fails for account menu in our integration.
  • Direct client-side request to the menu endpoint returns valid account menu data.
  • fetchMenu('account', ...) can still return undefined in the failing hard-refresh path (no explicit error), even with:
    • key: menu-account-${currentUserId}
    • credentials: 'include'
    • getCachedData: () => undefined

So:

  • The previous broad client $fetch fallback in fetchMenu() is still not ideal as a general fix.
  • But the revised “cache key / getCachedData only” change by itself does not fully fix the real hard-refresh behavior in our tested
    integration.

In other words, our earlier comment overstated that cache/key handling was the root cause. It appears to be part of the picture,
but not a complete resolution for this reproducible case.

Happy to continue on a maintainer-preferred direction for a proper module-level fix that preserves normal fetchMenu() behavior
without ad-hoc caller workarounds.

@fago

fago commented May 8, 2026

Copy link
Copy Markdown
Contributor

we could support safer behavior for user-dependent menus via an explicit key:

right, that seems to be a needed fix for the case where the user logs in during an active nuxt-instance. But is this even a relevant part for your scenario? As I understand, your account-menu is not correctly populated even on regular SSR page refresh if the user is logged-in. Correct?

If we can leave the "react upon user login" case out of the equation for now, that would help to keep things simpler. But that also means the user-ID is not needed as part of the cache-key, assuming we simply to a hard-reload of the whole page after login.

But the revised “cache key / getCachedData only” change by itself does not fully fix the real hard-refresh behavior in our tested integration.

Seems like this is something we need to track down still - the root cause. Why is empty when it should not be. Let's try to find the underlying issue.

@StirStudios
StirStudios force-pushed the fix/fetchmenu-hydration branch from 316f342 to 0d3182b Compare August 9, 2026 08:39
@StirStudios StirStudios changed the title fix(fetchMenu): fallback to $fetch on client after hydration feat(menu): support deferred lifecycle-safe loading Aug 9, 2026
@StirStudios

Copy link
Copy Markdown
Contributor Author

@fago We revisited this based on your concern that the problem was deeper than the original client-side fallback, and you were right.

The underlying issue is not the menu endpoint or Drupal proxy. It is when the Nuxt useFetch()-backed composable is first created. If fetchMenu() is first invoked from onMounted() during hydration, Nuxt queues its immediate execution in the component's beforeMount callbacks. Since beforeMount has already run, that request never starts, leaving the returned menu data undefined until a later navigation or render cycle.

The earlier $fetch fallback bypassed that lifecycle trap, but it also bypassed the normal AsyncData behavior and only treated the symptom. We have removed that approach and rebuilt the branch on the latest upstream 2.x.

This revision adds useMenu(), which exposes the underlying AsyncData result, including execute(). A consumer can now create the request during setup with immediate: false and server: false, then safely call execute() from onMounted(), a watcher, or an event. The existing fetchMenu() API remains intact and delegates to useMenu(), so existing setup-time callers are not broken.

We also added a deferred-execution regression test and documented the intended lifecycle-safe usage. This should address the underlying menu-loading issue while preserving Nuxt's standard data-fetching contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants