-
-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(nuxt): avoid injecting url helpers into globalThis
#9627
Conversation
Β Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
globalThis
globalThis
Although this will heal the issues similar to nuxt/nuxt#14372, both server and client instances are supposed to run in an isolated environment. I'm okay with such change since also helps with vercel-edge but I'm afraid it won't solve the multi-instance issue. Other things including hookable core depend on unique globalThis instance and it is shared. |
// @ts-ignore | ||
globalThis.__buildAssetsURL = buildAssetsURL | ||
// @ts-ignore | ||
globalThis.__publicAssetsURL = publicAssetsURL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can keep injecting to globalThis as well
packages/vite/src/server.ts
Outdated
@@ -24,11 +24,11 @@ export async function buildServer (ctx: ViteBuildContext) { | |||
return { relative: true } | |||
} | |||
if (type === 'public') { | |||
return { runtime: `globalThis.__publicAssetsURL(${JSON.stringify(filename)})` } | |||
return { runtime: `__publicAssetsURL(${JSON.stringify(filename)})` } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then here we can conditionally render either with globalThis or without based on auto imports being enabled.
globalThis
globalThis
π Linked issue
resolves nuxt/nuxt#14372
β Type of change
π Description
On server-side, rather than declaring
__buildAssetsURL
and__publicAssetsURL
onglobalThis
, this PR relies on nitro auto-imports to ensure they are injected where needed in the server app.Note: We would need to ensure that the nitro auto-imports functionality cannot be turned off if we are relying on it in this context. See also nitrojs/nitro#742.
π Checklist