⚠️ No Changeset found

Latest commit:5bc1a85

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go.If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

This looks really good. The main thing I noted while looking at this is that most Svelte users use Vite and the Vite docs say nothing aboutsideEffects. I took a quick look though and do see some references to it in the Vite code, so I'd imagine there's some level of support. It may be worth at least filing an issue with Vite noting the absence of this feature from their docs

Thanks! Sounds good, I'veopened an issue, we can see what they say.

I do see mention of side effects inrollup's docs, but I can't find it in Vite's either.

see this issue in vitevitejs/vite#4389 (via#10633)

A blanket recommendation for"sideEffects": false can break libraries that actually have side effects and rely on them to work (in my opinion svelte libraries generally should not, and explicitly listing the effectful files is better, but that isn't mentioned at all).

Before we change the documentation i'd like to see a repository demonstrating that and how it works.

Other options to explore:

1. Tagging the css module returned from vite-plugin-svelte load with moduleSideEffect: falsehttps://rollupjs.org/plugin-development/#load (although i'm not sure if/how vite handles these for css)

2. Not using a barrel index.js file to list all components, but an exports map that maps deep imports to root, so they are less cumbersome to use

"exports": {"svelte": {"import": {"./Foo.svelte":"./src/components/Foo.svelte"     }  }}

(I did experiment with globs in exports map a while ago but back then it was buggy for *.svelte)

in my opinion svelte libraries generally should not, and explicitly listing the effectful files is better, but that isn't mentioned at all

I agree they shouldn't, so think it would be nice to make that the recommendation. I think this PR both mentions the dangers of getting it wrong and shows how to explicitly list the files with side effects where it says "Make sure that"sideEffects" is correctly set. If a file with side effects is incorrectly marked as having no side effects, it can result in broken functionality. If your package has files with side effects, you can specify them in an array:"

Not using a barrel index.js file to list all components, but an exports map that maps deep imports to root, so they are less cumbersome to use

We could mention that would also address the issue in this section. I don't think we should go as far as recommending users always do it as it seems to be a case-by-case basis as to which might be the better design for a library

did some investigation here:sveltejs/vite-plugin-svelte#760

there are several ways in vite/rollup to say some module does not have a side effect, but currently the only one working seems to be the package.json field that's mentioned here. So unless there is a change that allows us to declare the virtual css modules from vite-plugin-svelte as free, the suggestion to add"sideEffects": ["your side effect files here"] to package.json of libraries is probably the best one.

If you are using barrel files inside your own application, you can also add this to the package.json in your app and it'll work.

Judging from the discussion in#10691 (comment) it seems that bundlers work in weird ways (and may work differently across bundlers?) depending on the value ofsideEffects. Until we got to the bottom of this I'll put this in draft mode (the text itself is well written though!)

Just throwing my hat into the ring here that this should at least be better advertised due to how focused Sveltekit is on performance. Right now I am maintaining a pretty mature Sveltekit app that imports a core Svelte library everywhere. Implementing this simple trick just gained me like 10 points on my lighthouse scores.

I'm marking this as ready for further review now that the issue I filed with Vite has been fixed and out for several releases.

I did a test on kit.svelte.dev:

current
du -ch .svelte-kit/output/client/_app/immutable/{entry,chunks}/.js | grep total
112Ktotal
du -ch .svelte-kit/output/client/_app/immutable/assets/.css | grep total
188Ktotal

with"sideEffects": ["**/*.css"] insite-kit
du -ch .svelte-kit/output/client/_app/immutable/{entry,chunks}/.js | grep total
104Ktotal
du -ch .svelte-kit/output/client/_app/immutable/assets/.css | grep total
172Ktotal

with"sideEffects": false insite-kit
du -ch .svelte-kit/output/client/_app/immutable/{entry,chunks}/.js | grep total
104Ktotal
du -ch .svelte-kit/output/client/_app/immutable/assets/.css | grep total
172Ktotal

thank you!! it was a long journey on this one, but I'm glad we got it figured out. I'll follow up with a PR to update the default increate-svelte to follow the best practice discussed here