Skip to content

fix(ssr)!: don't access Object variable in ssr transformed code #19996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hi-ogawa
Copy link
Collaborator

@hi-ogawa hi-ogawa commented May 3, 2025

Description

I added __vite_ssr_exportName__ to runtime context so we can move out Object.definedProperty from ssr transformed code. Since this breaks vite-node, I added a patch to add the same runtime context.

This is likely hard to test on ecosystem ci since anyone using Vitest / vite-node breaks 🤔
I made a Vitest side PR (and package) so they can be tested if both overrides are setup vitest-dev/vitest#7925.

As seen in ecosystem-ci below, this change didn't break vite-plugin-cloudflare nor vite-environment-examples because their custom module runner uses Object.keys(context) and Object.values(context) without referencing exact context keys https://github.com/cloudflare/workers-sdk/blob/1cd30a554f00dfd7bff43bbd3e601bc67f7acb2b/packages/vite-plugin-cloudflare/src/runner-worker/module-runner.ts#L55-L69

hi-ogawa added 2 commits May 3, 2025 15:13

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@hi-ogawa
Copy link
Collaborator Author

hi-ogawa commented May 3, 2025

/ecosystem-ci run

Copy link

pkg-pr-new bot commented May 3, 2025

Open in StackBlitz

npm i https://pkg.pr.new/vite@19996

commit: 8dd16df

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on 9ac01e3: Open

suite result latest scheduled
analogjs failure success
ladle failure success
nuxt failure success
laravel failure failure
quasar failure success
histoire failure success
storybook failure success
react-router ⏹️ cancelled success
sveltekit failure success
vike failure failure
unocss failure success
vite-plugin-svelte failure success
vite-plugin-react failure success
vite-plugin-vue failure success
vitepress failure success
waku failure success
vitest failure failure
vuepress failure success

astro, rakkas, previewjs, qwik, marko, vite-plugin-pwa, vite-plugin-react-swc, vite-environment-examples, vite-plugin-cloudflare, vite-setup-catalogue

@hi-ogawa hi-ogawa marked this pull request as ready for review May 3, 2025 08:47
@sapphi-red sapphi-red added this to the 7.0 milestone May 7, 2025
@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: ssr breaking change labels May 7, 2025
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

sapphi-red
sapphi-red previously approved these changes May 7, 2025

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change feat: ssr p3-minor-bug An edge case that only affects very specific usage (priority) trigger: preview
Projects
None yet
2 participants