mskocik / svelecte

Flexible autocomplete/select component written in Svelte,
https://svelecte.vercel.app
MIT License
447 stars 42 forks source link

What about Svelte 5? #225

Closed frederikhors closed 4 months ago

mskocik commented 4 months ago

when released

frederikhors commented 4 months ago

Is Release candidate starting today! https://youtu.be/gkJ09joGBZ4?t=8613

mskocik commented 4 months ago

Good to hear! Soon I will release patch version with updated svelte version peer-dependency.

There will also be a new major release, which will be Svelte 5 only. I plan to replace slots by snippets, etc. But it's hard to estimate when. Usually I do (timeconsuming) stuff like this when I have to stay in bed for a few days 😆

mskocik commented 4 months ago

Just to let you know... I just tried to update to svelte 5 and it completely broke the component. Multiple times and I have no time for this. Will wait for official migration tool and move from there. Not sure if v5-rewrite will be required or not.

frederikhors commented 4 months ago

Thanks.

mskocik commented 4 months ago

Let's keep this open to track the progress (and as a reminder for myself 😊)

james-em commented 4 months ago

Just to let you know... I just tried to update to svelte 5 and it completely broke the component. Multiple times and I have no time for this. Will wait for official migration tool and move from there. Not sure if v5-rewrite will be required or not.

I gave a try myself https://github.com/mskocik/svelecte/compare/master...james-em:svelte5?expand=1 No optimisation, just giving bare minimum to get it in a working state.

I've gotten a few page of the original website to work as expected, but the "Rendring" page is having a Maximum update depth exceeded.. Some stuffs around watch_* methods are causing infinite loop as everything is very reactive now and they seem to update the value reactive var. I believe watch_selectedOptions updates value, then watch_value_change gets called and calls clearSelection(); which redefine selectedOptions, which then watch_selectedOptions react again and so on.

Svelecte could benifit hugely from a full rewrite as the mentality behind the code is really really opposite to Svelte 5.

It's possible that for now this commit https://github.com/mskocik/svelecte/commit/7beb29f4a3e6cf73d7d0d0d76be9eb4221763e4c is in a usable state for Svelte 5 but it would use the old Svelte 4 way which is still supported at the moment. Could allow us folks to be more patient. Edit: It's not. Maximum update depth exceeded. is still present

mskocik commented 4 months ago

@james-em You just confirmed what I found out as well, when trying to update. But I was expecting they will fix reactivity in v5.

I will get into it soon.

james-em commented 4 months ago

@james-em You just confirmed what I found out as well, when trying to update. But I was expecting they will fix reactivity in v5.

I will get into it soon.

I believe it is an expected behavior and not a bug. Svelte 5 is able to pickup updates even without replacing the whole array or object.

Also the Svelte 5 documentation also state we should avoid as much as possible the use of $effect which I couldn't without rewriting a lot of logic.

I doubt the migration tool will be much of help in this this specific scenario.

Are you planning rewriting from the bottom up the library?

mskocik commented 4 months ago

I believe it is an expected behavior and not a bug. Svelte 5 is able to pickup updates even without replacing the whole array or object.

Yes, it is expected, but it wasn't fully working, which also Rich Harris admits himself.

Anyway I was expecting svelte 5 will break the current design, which uses/abuses current reactivity model, which isn't very reactive in some cases, or is slow to update.

Are you planning rewriting from the bottom up the library?

Yes, definitely. I prepared the ground for it v4 few months ago. Hopefully it will be quite easy or at least straighforward.

mskocik commented 4 months ago

Svelecte is now ready for svelte v5 under next tag.

npm i svelecte@next

@james-em thanks for your initial work with v5 update. I managed to make it work with v5 quite easily. More complete rewrite comming (hopefully) soon.

mskocik commented 4 months ago

@james-em in the latest release I fixed the issue which caused the endless loop error resulting in maximum update depth exceeded. This release should be fully compatible with svelte 5 and still have the same functionality as v4.

jamesst20 commented 4 months ago

@james-em in the latest release I fixed the issue which caused the endless loop error resulting in maximum update depth exceeded. This release should be fully compatible with svelte 5 and still have the same functionality as v4.

Thanks for the feedback! I will definitely look into it! (james-em is my business github account)

jamesst20 commented 4 months ago

@mskocik

You have defined new snipped props but they are marked as required making it a little hard on the use :)

  // snippets
  export let prepend;
  export let append;
  export let listHeader;
mskocik commented 4 months ago

ups, I fixed that, but it as I was experimenting on multiple fronts it got discarded as well 🙂

mskocik commented 4 months ago

@jamesst20 fixed now