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

breaking: drop support for svelte4, remove unused code and update types #892

Merged
merged 19 commits into from
May 3, 2024

Conversation

dominikg
Copy link
Member

vite-plugin-svelte v3 supports both svelte4 and svelte5.

Due to breaking changes in the svelte compiler, around hmr and for how we add svelte-inspector, it is not feasible to support both svelte majors from the same v-p-s major.

Users on svelte4 must continue to use vite-plugin-svelte@3

This PR makes the grunt work of changes to remove svelte4 related code. Expect some tests to fail intially

@dominikg
Copy link
Member Author

note to self: remove ts transform from vitePreprocess

@dominikg
Copy link
Member Author

note to self: check support for lightningcss and css nesting syntax

@dominikg dominikg added this to the 4.0 milestone Apr 30, 2024
@Conduitry
Copy link
Member

Conduitry commented Apr 30, 2024

The linting errors about $foo is an illegal variable name appear to be a bug in the ESLint plugin.

As I've mentioned here, it appears these can be worked around by adding additional references to the stores, like so:

diff --git a/packages/playground/kit-demo-app/src/routes/Header.svelte b/packages/playground/kit-demo-app/src/routes/Header.svelte
index 368b721e..454e1e23 100644
--- a/packages/playground/kit-demo-app/src/routes/Header.svelte
+++ b/packages/playground/kit-demo-app/src/routes/Header.svelte
@@ -1,5 +1,6 @@
 <script>
 	import { page } from '$app/stores';
+	page;
 	import logo from '$lib/images/svelte-logo.svg';
 	import github from '$lib/images/github.svg';
 </script>
diff --git a/packages/playground/kit-demo-app/src/routes/sverdle/+page.svelte b/packages/playground/kit-demo-app/src/routes/sverdle/+page.svelte
index 2784431d..ce7bb67d 100644
--- a/packages/playground/kit-demo-app/src/routes/sverdle/+page.svelte
+++ b/packages/playground/kit-demo-app/src/routes/sverdle/+page.svelte
@@ -3,6 +3,7 @@
 	import { enhance } from '$app/forms';
 
 	import { reduced_motion } from './reduced-motion';
+	reduced_motion;
 
 	/** @type {import('./$types').PageData} */
 	export let data;
diff --git a/packages/playground/optimizedeps-include/src/App.svelte b/packages/playground/optimizedeps-include/src/App.svelte
index ccbba8da..5ce4e7de 100644
--- a/packages/playground/optimizedeps-include/src/App.svelte
+++ b/packages/playground/optimizedeps-include/src/App.svelte
@@ -1,5 +1,6 @@
 <script>
 	import { Route, router } from 'tinro';
+	router;
 </script>
 
 <nav>

This does, then, leave me with a bunch of other type errors still present locally.

I don't know whether it makes sense to apply a workaround like the above for now. I don't know what the expected timeline is on dropping Svelte 4 support here and on fixing the eslint-plugin-svelte bug.

@dominikg
Copy link
Member Author

as for the timeline, right before/around svelte5 final release, the eslint bug needs fixing too, users would rightfully be royally annoyed by this i

@@ -19,33 +19,24 @@ export async function loadRaw(svelteRequest, compileSvelte, options) {
try {
//avoid compileSvelte doing extra ssr stuff unless requested
//@ts-ignore //@ts-expect-error generate value differs between svelte4 and 5
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we can also remove this

"@sveltejs/vite-plugin-svelte-inspector": "^2.1.0",
"@sveltejs/vite-plugin-svelte-inspector": "workspace:^",
Copy link
Member

Choose a reason for hiding this comment

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

I guess we want to use the static ^3.0.0 after it's published?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, either that, or inline the inspector into v-p-s entirely, or remove the dependency and let users add it to their vite config manually. The current situation is a bit tedious for maintenance

"@sveltejs/vite-plugin-svelte": "^3.0.0",
"svelte": "^4.0.0 || ^5.0.0-next.0",
"@sveltejs/vite-plugin-svelte": "workspace:^",
Copy link
Member

Choose a reason for hiding this comment

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

And here (just to highlight)

@dominikg dominikg merged commit 49324db into main May 3, 2024
6 checks passed
@dominikg dominikg deleted the breaking/drop-svelte4 branch May 3, 2024 18:57
@github-actions github-actions bot mentioned this pull request May 3, 2024
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