Skip to content
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

fix(ssr): inject identity function at the top #18449

Merged

Conversation

sapphi-red
Copy link
Member

Description

This PR should fix marko's fail in #9981 (comment).

@sapphi-red sapphi-red added p4-important Violate documented behavior or significantly improves performance (priority) feat: ssr labels Oct 24, 2024
Comment on lines +1196 to +1204
"const __vite_ssr_identity__ = v => v;
const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["foo"]});


__vite_ssr_exports__.default = __vite_ssr_identity__(__vite_ssr_import_0__.foo)()
const __vite_ssr_import_1__ = await __vite_ssr_import__("./bar");

Object.defineProperty(__vite_ssr_exports__, "bar", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_1__ }});
console.log(bar)
Copy link
Member Author

@sapphi-red sapphi-red Oct 24, 2024

Choose a reason for hiding this comment

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

The output without the fix is:

const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["foo"]});


__vite_ssr_exports__.default = __vite_ssr_identity__(__vite_ssr_import_0__.foo)()
const __vite_ssr_import_1__ = await __vite_ssr_import__("./bar");
const __vite_ssr_identity__ = v => v;

Object.defineProperty(__vite_ssr_exports__, "bar", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_1__ }});
console.log(bar)

Note that __vite_ssr_identity__ is called before declaration.

@sapphi-red
Copy link
Member Author

/ecosystem-ci run

bluwy
bluwy previously approved these changes Oct 24, 2024
@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on 0c743b5: Open

suite result latest scheduled
astro failure failure
previewjs failure success
redwoodjs failure failure
remix failure failure
sveltekit failure failure
vike failure failure
vite-plugin-svelte failure failure
vitest failure failure

analogjs, histoire, ladle, laravel, marko, nuxt, quasar, qwik, rakkas, storybook, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-vue, vite-setup-catalogue, vitepress, vuepress

@sapphi-red
Copy link
Member Author

sapphi-red commented Oct 24, 2024

Vike had a similar error (https://github.com/vitejs/vite-ecosystem-ci/actions/runs/11496916712/job/31999518321#step:8:1969) and that is fixed by this PR too.

@patak-dev patak-dev enabled auto-merge (squash) October 24, 2024 11:58
@patak-dev patak-dev merged commit 0ab20a3 into vitejs:main Oct 24, 2024
14 checks passed
@sapphi-red sapphi-red deleted the fix/inject-identity-function-at-the-top branch October 24, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ssr p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants