Skip to content

fix(fetchMenu): fallback to $fetch on client after hydration#461

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

fix(fetchMenu): fallback to $fetch on client after hydration#461
StirStudios wants to merge 2 commits into
drunomics:2.xfrom
StirStudios:fix/fetchmenu-hydration

Conversation

@StirStudios
Copy link
Copy Markdown
Contributor

@StirStudios StirStudios commented Mar 20, 2026

Summary

  • update fetchMenu flow to use a client-safe $fetch fallback after hydration
  • keep server-side behavior unchanged while preventing client-side hydration path failures

Why

  • resolves cases where hydrated client navigation can fail to retrieve menu data through the server-oriented path
  • ensures menu requests continue to work consistently across SSR and post-hydration client execution

Scope

  • targeted change in src/runtime/composables/useDrupalCe/index.ts
  • no public API changes

Validation

  • focused on hydration/menu-fetch behavior regression addressed by this patch

@fago
Copy link
Copy Markdown
Contributor

fago commented Apr 14, 2026

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
Copy link
Copy Markdown
Contributor

fago commented May 6, 2026

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
Copy link
Copy Markdown
Contributor

fago commented May 8, 2026

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.

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