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: set scripts imported by HTML moduleSideEffects=true #18411

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sapphi-red
Copy link
Member

Description

When setting moduleSideEffects: false, all scripts gets tree-shaked and the built output becomes empty. This is because we convert HTML files into a JS file just containing side effect imports that points to the href attr of script tags.

While the behavior is correct (the scripts do have a side-effect and setting it false is a lie) and kind of expected, it is more easier to configure if these scripts are marked as moduleSideEffects: true as in most cases you won't rely on side-effect imports. (The correct way of fixing this is to set treeshake.moduleSideEffects: (id) => scriptsImportedByHtml.includes(id) instead.)

fixes #16720
fixes #17911

This PR reverts #12786 as this fix covers that one as well. (To fix that one correctly, the sideEffects field should include the scripts imported by the HTML file.)

@sapphi-red sapphi-red added the p2-to-be-discussed Enhancement under consideration (priority) label Oct 21, 2024
@patak-dev
Copy link
Member

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on cb81d59: Open

suite result latest scheduled
analogjs failure success
astro failure failure
remix failure failure
sveltekit failure failure
vike failure failure
vite-environment-examples success failure
vite-plugin-svelte failure failure
vitest failure failure

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-to-be-discussed Enhancement under consideration (priority)
Projects
None yet
2 participants