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

Prebundling of libraries with Svelte components #2612

Closed
benmccann opened this issue Oct 15, 2021 · 15 comments · Fixed by #7760
Closed

Prebundling of libraries with Svelte components #2612

benmccann opened this issue Oct 15, 2021 · 15 comments · Fixed by #7760
Assignees
Labels
feature / enhancement New feature or request p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. size:large significant feature with tricky design questions and multi-day implementation vite
Milestone

Comments

@benmccann
Copy link
Member

benmccann commented Oct 15, 2021

Describe the bug

See illright/attractions#311 (comment) for the original report

It was loading all of svelte-feather-icons when you use just a few icons. Vite is supposed to do dependency pre-bundling to avoid this: https://vitejs.dev/guide/dep-pre-bundling.html#the-why. However, Vite's pre-bundling uses esbuild, so we disabled it on Svelte components because it doesn't know what to do with .svelte files.

Evan has proposed changing this. I'm finally a convert to the idea.

Reproduction

Try to use svelte-feather-icons

Logs

https://github.com/illright/attractions/ was making 485 requests for each page in dev mode taking 10s to load

System Info

Vite 2.6.7

Severity

serious, but I can work around it

Additional Information

You can work around it by doing deep imports

@benmccann benmccann added this to the 1.0 milestone Oct 15, 2021
@benmccann benmccann changed the title Large Svelte libraries cause terrible performance Large Svelte libraries cause terrible performance in dev mode Oct 15, 2021
@antony
Copy link
Member

antony commented Oct 15, 2021

This was the same in Sapper. Reloads were taking 60 seconds. I made a reduced version of the library including only the icons we required, and reloads started taking 20 seconds. Game changer.

I suppose what this means is that, the library isn't properly tree-shakeable.

@benmccann
Copy link
Member Author

I don't think there's anything we can change in the library or is related to tree shaking, but requires changes in Vite. I updated the description with more details

@dominikg
Copy link
Member

Until vite can pre-bundle svelte libraries it's best to use deep imports to avoid paying the tax of loading all exported svelte components.

Icon libraries are usually the worst in this context, luckily https://github.com/antfu/unplugin-icons does not suffer from this.

@techniq
Copy link
Contributor

techniq commented Oct 16, 2021

I've seen some discussion in carbon-components-svelte to preprocess the base imports to the individual full source Svelte code paths, which looks like it should help.

I've been meaning to try this approach myself for some libraries I've been working on but so far it hasn't been an issue enough to try. For icons I typically use @mdi/js for the icon paths and have a single Icon component to setup the SVG viewport/etc (ex. <Icon path={mdiSomething} />). Adding @mdi/js to optimizeDeps.include seems to make it performance tolerable so far.

@benmccann
Copy link
Member Author

Wow, that's a great solution! Yeah, I wonder if we could tweak that library and use it generally. I've asked the author in carbon-design-system/carbon-preprocess-svelte#20

@dominikg
Copy link
Member

@bluwy didn't we discuss something like that before? Main problem i see is that a generic preprocess-use-deep-import would have to resolve the intended target on the spot, following multiple reexports in the worst case

@bluwy
Copy link
Member

bluwy commented Oct 16, 2021

I think we discussed something similar too about transforming the entrypoint with only the named imports, so it works more generically. The preprocessor technique is interesting though, I suppose it can be worked around by checking the exports field and naively work with that.

Anywho, I've been working today on actually prebundling Svelte libraries. It seems to be working on this branch. Wish I thought about it sooner 😄 The caveats now are that:

  1. Prebundling would take a while (6s for carbon-components-svelte), but that's negligible in the long run.
  2. Deeply importing Svelte components is not compatible, it could cause multiple instance of carbon-components-svelte running. Unless we force prebundle that nested import too. Could be a bug in Vite.
  3. The generated sourcemap causes Vite to log "Unrestricted file system access". Could be an implementation error.

In that branch, I have a big-component-library playground to test out. I'd appreciate if anyone likes to test this in their project as well.

@benmccann
Copy link
Member Author

I don't think the prebundling delay is too much of a big deal since it's only the first time you start the dev server and doesn't happen on subsequent runs.

Adding a link to the merged PR for the new feature for reference: sveltejs/vite-plugin-svelte#200

@bluwy
Copy link
Member

bluwy commented Oct 18, 2021

Since 1.0.0-next.185, you can set experimental.prebundleSvelteLibraries: true in svelte.config.js to test out the new prebundling handling. Feedback appreciated!

@benmccann
Copy link
Member Author

Vite 2.9 will add better support for prebundling Svelte libraries: vitejs/vite#6801

@benmccann benmccann added feature / enhancement New feature or request vite labels Mar 17, 2022
@benmccann benmccann changed the title Large Svelte libraries cause terrible performance in dev mode Prebundling of libraries with Svelte components Mar 17, 2022
@bluwy
Copy link
Member

bluwy commented Jul 5, 2022

I haven't got any negative feedback so I hope all's good with prebundleSvelteLibraries 😄 I'll try to get this stabilized once Vite 3 lands. There's a lot of moving parts with the optimizer in Vite 3, so there's a chance prebundleSvelteLibraries needs tweaks for it to work later.

@benmccann benmccann added p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. size:large significant feature with tricky design questions and multi-day implementation labels Jul 30, 2022
@benmccann
Copy link
Member Author

Since vite-plugin-svelte 1.0 this option can now be set with the following in svelte.config.js:

  vitePlugin: {
    experimental: {
      prebundleSvelteLibraries: true
    }
  }

@benmccann
Copy link
Member Author

The main thing to do before graduating from experimental is probably sveltejs/vite-plugin-svelte#419

@ZetiMente
Copy link

ZetiMente commented Oct 9, 2022

@bluwy Been using vitePlugin: { experimental: { useVitePreprocess: true }} for a fairly large web app & it has been great ! Except the annoying messages in the build, there has been no issues. It also handles the css background image url and a few other things better. And it allows you to remove Svelte preprocess from your package ! Thanks !

@dominikg
Copy link
Member

SvelteKit currently has prebundleSvelteLibraries forcefully enabled via inline config to vite-plugin-svelte.

We are still working on enabling it by default in vite-plugin-svelte, which should happen in it's next feature release. See sveltejs/vite-plugin-svelte#494

After that SvelteKit can remove that setting from it's own code and users will be able to disable it completely if they so desire.
See sveltejs/vite-plugin-svelte#494 (comment) for a preliminary explaination of prebundling svelte libraries, index imports vs deep imports and large component libraries and why it isn't always the best to prebundle. vite-plugin-svelte docs are going to explain it in more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. size:large significant feature with tricky design questions and multi-day implementation vite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants