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] enable Vite pre-bundling except for Svelte packages #2137

Merged
merged 2 commits into from Aug 9, 2021

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented Aug 9, 2021

This is part of why you can't use any Svelte packages with CJS dependencies (vitejs/vite#3024), which is probably the top issue we encounter using Vite.

We need to exclude Svelte packages due to vitejs/vite#3910. Pnpm users may need to either (1) set pnpm shamefully-hoist=true or (2) install that dependency in package.json so we can import it.

This PR alone doesn't actually fix much due to vitejs/vite#3910. However, it does allow me to make more progress in working on Vite issues because it allows me to use the kit.vite.optimizeDeps options. When optimizeDeps is set to [] then none of the other options work and there's no way to unset that from the config. Since it shouldn't actually be set anyway, let's go ahead and remove it

@benmccann benmccann added bug Something isn't working vite labels Aug 9, 2021
@benmccann benmccann requested a review from dominikg August 9, 2021 00:25
@changeset-bot
Copy link

changeset-bot bot commented Aug 9, 2021

🦋 Changeset detected

Latest commit: 2fc2654

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

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

@dominikg
Copy link
Member

dominikg commented Aug 9, 2021

as mentioned in sveltejs/vite-plugin-svelte#125 i think this is not something limited to sveltekit and having it in vite-plugin-svelte would make it more useful.

@benmccann
Copy link
Member Author

I'm totally happy to have it live there. I'll go ahead and merge this for now since we'd also want to move ssr.noExternal. When we get the code into vite-plugin-svelte we can remove it from here

@AaronDDM
Copy link

@benmccann not sure if the subsequent change you plan on making will fix this but after this PR was merged - I am getting errors like the following during server build time:

vite v2.4.4 building SSR bundle for production...
...index.svelte:5:14: error: Expected "from" but found "{"
    5 │   import type { Application } from "../../types/Application";

And it's because this line was removed:
https://github.com/sveltejs/kit/pull/2137/files#diff-09b70580810ae66bf783d467b3ea4f54cd1ac4cb57532b38b788145e27cc0983L581

Which I assume enabled optimization of deps.

Not sure if anyone else is running into this, but I've manually gone in and re-introduced the entries: [] property and the errors go away.

@benmccann
Copy link
Member Author

See sveltejs/svelte-preprocess#362 (comment) for more details about that issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants