fix(nuxt): clone app config on server

[danielroe]

❓ Type of change

• 📖 Documentation (updates to the documentation or readme)
• 🐞 Bug fix (a non-breaking change that fixes an issue)
• 👌 Enhancement (improving an existing functionality like performance)
• ✨ New feature (a non-breaking change that adds functionality)
• 🧹 Chore (updates to the build process or auxiliary tools and libraries)
• ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Calling reactive on an object mutates the object, so previously we were causing a memory leak by mutating app config (as the underlying object was shared between requests). Instead, this PR clones the object on server side - and does not make it reactive. This might help with #20056 (although I can't verify for sure as there is no reproduction).

Additionally, as app config is shared across requests anyway, I don't believe we need to set it on NuxtApp as well. This frees us up to use app config ambiently, not necessarily in a plugin/setup context. We can also somewhat reduce the implementation size by getting rid of a workaround for Vite HMR.

I would welcome context/discussion if you think differently - an alternative solution would be to use structuredClone or equivalent + not calling reactive on the app config.

📝 Checklist

• I have linked an issue or discussion.
• I have updated the documentation accordingly.

[codesandbox]

  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

[atinux]

I like it. We don’t need to make it reactive on SSR either to attach to nuxtApp instance

[DamianGlowala]

I have a production app running on Nuxt 3.2.3. Happy to update to the latest edge once merged, deploy it and report whether the memory leaks can still be observed.

[danielroe]

Thank you! You might also consider using pnpm patch or patch-package to try out these changes before that point. (I hope to enable edge commits for PRs on-demand later on... 🤔 )

[pi0]

Nice find but appConfig is not supposed to be shared across requests same. One of the main fundamental goals is that it can be modified per-request basis so it needs to be bound to nuxt app instance. Making things like remote-config possible but not limited to this only.

We might try structured cloning or simply a deep clone on first edit (via a proxy)

still in discussion

[danielroe]

Since originally suggesting structuredClone I've changed my mind a little. Here's why:

1. We currently expose useAppConfig as usable in Nitro routes outside the event/request context. I think keeping DX consistent between Nitro + Nuxt means that we need to allow this kind usage.

2. Apart from that, I think it would be strange if we cannot access this 'build-time' config outside of a request - it means it can't be used for setting properties in Vue component defaults, or initialising global defaults. That seriously limits its usability, for example in themes.

Having said that, I think we have two options. (More ideas welcome.)

1. Single composable. We keep useAppConfig compatible with Nitro-style usage (be usable without referencing event) and equalise Nuxt behaviour so that it is the same - this config can be called and used in ambient JS. When this is called in ambient JS or outside of a request context, it is deep-frozen (on the server) just like Nitro. When it is called within a Vue/Nuxt request context, then it is automatically cloned, attached to Nuxt instance and made reactive.

2. New composable. We introduce a new function useBaseAppConfig which allows accessing (but not setting) the 'raw' app config. This is always deep-frozen on the server and can be used 'ambiently' outside of the request context. Alternatively, we add a new composable useRequestAppConfig which gives access to a reactive app config that is bound to the request.

Thoughts welcome.

[pi0]

Regarding nitro, I'm thinking to also make it also stricter to require composable utils like useStorage and useAppConfig (also useRuntimeConfig) to be used within event handlers. not outside them because platforms such as cloudflare modules only provide environment within the context and that's why there are a callable composable not a direct export like { nuxtAppConfig }. The current code you shared is going to be changed (very soon!)

We could fall back to the frozen version of the app config-if nuxt app context is not available to avoid introducing another utility but i would even make a warning for that. App config logic should be context aware. Wdyt?

[pi0]

(discussion to be resolved it is not stale)

still in discussion

[danielroe]

Updated (and following unjs/nitro#1154 to use klona on server). Once that's merged, we can align the implementations further...

[pi0]

LGM. Nice solution 💯