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

feat: enable prebundling by default #494

Merged
merged 16 commits into from Nov 22, 2022
Merged

Conversation

dominikg
Copy link
Member

@dominikg dominikg commented Nov 12, 2022

enable prebundleSvelteLibraries by default

TODO:

  • merge fix: preserve reincludes of excluded with prebundleSvelteLibraries #493
  • release vite-plugin-svelte patch so a fixed version with default disabled exists in the registry
  • add logging for prebundling stats and dev ssr compile time to increase visibility for page load time issues during dev
  • add documentation about tradeoffs for prebundling, deep imports vs index imports, build and ssr.
  • merge this PR and release vite-plugin-svelte minor
  • remove inline setting of prebundleSvelteLibraries: true from kit and bump it's minimum v-p-s version

@dominikg dominikg changed the title Feat/enable prebundling by default feat: enable prebundling by default Nov 12, 2022
.changeset/calm-rules-push.md Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
packages/vite-plugin-svelte/src/utils/options.ts Outdated Show resolved Hide resolved
packages/vite-plugin-svelte/src/utils/options.ts Outdated Show resolved Hide resolved
@dominikg
Copy link
Member Author

dominikg commented Nov 17, 2022

The guides and recommendations likely have to be rewritten as there are several factors at play.

A) optimizeDeps currently does not work for ssr, leading to slow load times during dev when you import a large library without using deep imports
B) using a preprocessor that rewrites imports from index import to deep import causes a massive problem and must be avoided when prebundling is enabled for that library ( eg carbon-preprocess-svelte optimizeImports preprocessor )
C) index imports still have an effect on build time. All components exported from an index have to be compiled, even if your Application only uses a few.

There is no golden rule for what's right, just general recommendations:

  1. don't use an import rewriting preprocessor together with prebundleSvelteLibraries
  2. deep imports avoid compiling unused .svelte files which can speed up dev ssr and build times esp. for large libraries
  3. index imports provide a better DX by enabling autocomplete even for previously unused components

picking between 2 and 3 has to be done by the users, v-p-s cannot decide that.

To improve performance for index imports we can look into enabling optimizeDeps for build and ssr too, but unfortunately i havn't been able to get it running for ssr so far.

docs/faq.md Outdated Show resolved Hide resolved
dominikg and others added 2 commits November 22, 2022 00:00
@dominikg dominikg marked this pull request as ready for review November 22, 2022 08:12
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

The docs are organized well for me. Have some nits below:

docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
@dominikg dominikg merged commit 8d39420 into main Nov 22, 2022
@dominikg dominikg deleted the feat/enable-prebundling-by-default branch November 22, 2022 12:39
@github-actions github-actions bot mentioned this pull request Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants