Migrate to Svelte 5 #716

kodaicoder · 2024-11-20T08:40:22Z

It's time for Svelte5 😆

I has re-writing a Select component and all +page.svelte to using a Svelte 5 rune mode so many $effect has been used without using any $derived that should been reviewing and change in the future if it not affected a core function

Issue waiting to solve

Even the core function working great but it still have some issue that need attention

on multi-item-checkboxes page there will be some infinite looping happen on $effect after selecting 1 of a items
onvirtual-list page there will be problem because svelte-tiny-virtual-list package still using svelte 4 then it can not passing snippet to render in there children , for now it will not show any error but the visualize will not show a list of items to be picked.
a problem that list of Select component not scrolling to a selected item
I didn't change README.md too then please using on Website as a document for now.

BTW this PR still have some rough edge and need a community to make it shine✨

vercel · 2024-11-20T08:40:31Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
svelte-select	❌ Failed (Inspect)			Nov 20, 2024 8:49am

kodaicoder · 2024-11-20T08:45:45Z

Oh, and another thing
I just has adding a most request function of the select that will made justValue
two way bindable and working as setter of data
you can see in use-just-value page

thank you for #641 for a idea on that

go-aegian · 2024-11-20T10:28:09Z

src/lib/filter.js

@@ -26,7 +28,6 @@ export default function filter({
                return filterSelectedItems ? x[itemId] === item[itemId] : false;


This can be simplified

Current:

matchesFilter = !value.some((x) => { return filterSelectedItems ? x[itemId] === item[itemId] : false; });

To:
matchesFilter = !value.some((x) => filterSelectedItems && x[itemId] === item[itemId]);

go-aegian · 2024-11-20T10:35:09Z

src/routes/examples/advanced/multi-item-checkboxes/+page.svelte

+    });
+    $effect(() => {
+        computeIsChecked(checked);
+    });


Check if these can be changed to use $derived states without any warnings or errors, then you won't have $effect at all and no infinite loops

Current:

let value = $state([]); let checked = $state([]); let isChecked = $state({}); $: computeValue(checked); $: computeIsChecked(checked); $effect(() => { computeValue(checked); }); $effect(() => { computeIsChecked(checked); }); function computeIsChecked() { isChecked = {}; checked.forEach((c) => (isChecked[c] = true)); } function computeValue() { value = checked.map((c) => items.find((i) => i.value === c)); }

To

let checked = $state([]); let value = $derived.by(()=> checked.map((c) => items.find((i) => i.value === c))); let isChecked = $derived.by(()=> { let r = {}; checked.forEach((c) => (r[c] = true)) return r; });

The cause of the infinite loop does not coming from a $effect that using on a +page.svelte because even I try comment out both of $effect in +page.svelte a issue of infinite loop still persist.

but dont be worry I will try tracking on this issue because it a last page that svelte-select should work 😁

Well either way still it might help reduce lines of code and complexity, if what I proposed works as expected

go-aegian · 2024-11-20T10:38:40Z

src/routes/examples/advanced/virtual-list/+page.svelte

-                </div>
+    onhoverItem={(e) => handleHover(e.detail)}>
+    {#snippet listSnippet(filteredItems)}
+        {#if filteredItems && filteredItems.length > 0}


You could simplify this

Current:

{#if filteredItems && filteredItems.length > 0} <VirtualList width="100%" height={200} itemCount={filteredItems.length} itemSize={30}>

To:

{@const itemCount = filteredItems?.length ?? 0; {#if itemCount > 0} <VirtualList width="100%" height={200} {itemCount} itemSize={30}>

go-aegian · 2024-11-20T11:05:33Z

I added some minor reviews if it helps

kodaicoder · 2024-11-20T11:16:09Z

@go-aegian latest reply is that a svelte-select project ?
I think it a svelte-flatpickr (datepicker) project.
🤔

go-aegian · 2024-11-20T11:26:21Z

@go-aegian latest reply is that a svelte-select project ? I think it a svelte-flatpickr (datepicker) project. 🤔

You are right sorry I got confused as I had trouble with that library too. Svelte 5 causing problems in the community for sure.

webJose · 2024-12-03T23:09:13Z

src/lib/tailwind.css

@@ -74,7 +74,7 @@
 }

 .list .list-group-title {
-    @apply text-gray-400 cursor-default text-sm font-medium h-10 leading-10 px-5 overflow-ellipsis whitespace-nowrap uppercase;
+    @apply text-slate-800 cursor-default text-sm font-medium h-10 leading-10 px-5 overflow-ellipsis whitespace-nowrap uppercase;


Regardless of whether this is a better choice than the other gray, you should not sneak changes like this because this PR is just for Svelte v5, right?

This change can be discussed and treated outside of the Svelte v5 migration context.

webJose · 2024-12-03T23:15:00Z

src/lib/Select.svelte

+            if (inputAttributes || !searchable) assignInputAttributes();
+        });
+    });
+    $effect(() => {


Suggested change

$effect(() => {

onMount(() => {

An effect that doesn't track anything is just going to run once, when mounting.

webJose · 2024-12-03T23:17:20Z

src/lib/Select.svelte

+        untrack(() => (hasValue = multiple ? value && value?.length > 0 : !!value));
+    });
+    $effect(() => {
+        hideSelectedItem = hasValue && filterText.length > 0;


hideSelectedItem can be a $derived.

webJose · 2024-12-03T23:17:57Z

src/lib/Select.svelte

+        hideSelectedItem = hasValue && filterText.length > 0;
+    });
+    $effect(() => {
+        showClear = hasValue && clearable && !disabled && !loading;


showClear can be a $derived.

webJose · 2024-12-03T23:18:23Z

src/lib/Select.svelte

+        showClear = hasValue && clearable && !disabled && !loading;
+    });
+    $effect(() => {
+        ariaSelection = value ? handleAriaSelection(multiple) : '';


ariaSelection can be a $derived.

webJose · 2024-12-03T23:19:33Z

src/lib/Select.svelte

+        ariaSelection = value ? handleAriaSelection(multiple) : '';
+    });
+    $effect(() => {
+        ariaContext = handleAriaContent({ filteredItems, hoverItemIndex, focused, listOpen });


ariaContext has no use. This is either a mistake, or is really not needed. If needed, it can be a $derived.

webJose · 2024-12-03T23:21:40Z

src/lib/Select.svelte

+        if (listOpen && container && list) setListWidth();
+    });
+    $effect(() => {
+        scrollToHoverItem = hoverItemIndex;


scrollToHoverItem seems unused right now. Its former usage is commented now. Error? Not needed anymore? Also this is weird. Can't its job be done with hoverItemIndex? Why this copy?

webJose · 2024-12-03T23:23:26Z

src/lib/Select.svelte

+        oninput = () => {},
+        onfilter = () => {},
+        onclear = () => {},
+        onfocus = () => {},
+        onchange = () => {},
+        onselect = () => {},
+        onblur = () => {},
+        onhoveritem = () => {},
+        onerror = () => {},
+        onloaded = () => {},


Let these be undefined, and then in code use ?.() to call them. Example: onfocus?.().

webJose · 2024-12-03T23:24:39Z

src/lib/Select.svelte

+        onhoveritem = () => {},
+        onerror = () => {},
+        onloaded = () => {},
+        ...rest


This is only used to carry rest.class. I'd just remove and simply add the class prop.

webJose · 2024-12-03T23:26:19Z

src/lib/Select.svelte

        if (disabled) return;
-        if (filterText.length > 0) return listOpen = true;
+        if (filterText && filterText.length > 0) return (listOpen = true);


Suggested change

if (filterText && filterText.length > 0) return (listOpen = true);

if (filterText?.length ?? 0 > 0) return (listOpen = true);

As it is, while the result is unchanged, incorrectly filters out the empty string value, to which one can query for its length.

nutchyleo added 3 commits November 13, 2024 16:01

update dependencies

cbec84e

refactor : migrate to svelte 5

b366297

fix: error on snippet data

f705262

vercel bot had a problem deploying to Preview November 20, 2024 08:40 Failure

fix :remove un-used select component

8ea9b29

vercel bot had a problem deploying to Preview November 20, 2024 08:49 Failure

kodaicoder mentioned this pull request Nov 20, 2024

Support for Svelte 5 #712

Open

go-aegian reviewed Nov 20, 2024

View reviewed changes

webJose suggested changes Dec 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to Svelte 5 #716

Migrate to Svelte 5 #716

kodaicoder commented Nov 20, 2024 •

edited

Loading

vercel bot commented Nov 20, 2024 •

edited

Loading

kodaicoder commented Nov 20, 2024

go-aegian Nov 20, 2024

go-aegian Nov 20, 2024

kodaicoder Nov 20, 2024

go-aegian Nov 20, 2024

go-aegian Nov 20, 2024

go-aegian commented Nov 20, 2024 •

edited

Loading

kodaicoder commented Nov 20, 2024

go-aegian commented Nov 20, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

webJose Dec 3, 2024

		@@ -26,7 +28,6 @@ export default function filter({
		return filterSelectedItems ? x[itemId] === item[itemId] : false;

	if (filterText && filterText.length > 0) return (listOpen = true);
	if (filterText?.length ?? 0 > 0) return (listOpen = true);

Migrate to Svelte 5 #716

Are you sure you want to change the base?

Migrate to Svelte 5 #716

Conversation

kodaicoder commented Nov 20, 2024 • edited Loading

It's time for Svelte5 😆

Issue waiting to solve

vercel bot commented Nov 20, 2024 • edited Loading

kodaicoder commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

go-aegian commented Nov 20, 2024 • edited Loading

kodaicoder commented Nov 20, 2024

go-aegian commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodaicoder commented Nov 20, 2024 •

edited

Loading

vercel bot commented Nov 20, 2024 •

edited

Loading

go-aegian commented Nov 20, 2024 •

edited

Loading