-
Notifications
You must be signed in to change notification settings - Fork 24
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!: migrate to Svelte 5 #145
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Would be amazing to see this merged! |
Until then, you can use this: https://www.npmjs.com/package/@eirikk/portabletext-2-svelte-5 Follow the instructions in the README to not have to rewrite any imports |
Anything I can do to help get this merged? It is disappointing that this has taken so long considering Svelte 5 has been out now for a considerable amount of time. |
Hello, we're releasing some significant updates for Svelte shortly. I'll get this reviewed and merged by the end of the week. Thanks for your patience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this, I've done an initial pass and it all looks pretty good to me.
I've just made some suggestions to primarily reduce intermediate $derived
values, but I don't think this is critical. I'm by no means an expert in Svelte's reactivity system so there could be good reason for them, let me know what you think either way and I'll get this approved, merged and released asap.
Also, I've run benchmarks against these suggested changes (I'll add the benchmark code in another PR) as I wondered if the intermediate $derived
values were performance optimisations, but I haven't been able to discern any meaningful difference. So my suggestions are largely addressing perceived readability, this is subjective so I'll leave it up to you whether or not you want to accept them.
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
Co-authored-by: Rupert Dunk <rupert@rupertdunk.com>
…ortabletext into svelte-5-migrate
Great! I accepted all suggestions. All the |
Awesome, thank you 🙇 . If you wouldn't mind rebuilding the |
Description
This is my attempt to kickstart svelte 5 support. It's probably enough to just allow svelte 5 as a peerDependency considering svelte 5 has backwards compatibility, but I guess it's nice to have this as a starting point for a full migration for projects that use runes mode only.
All tests are passing, though I had to add a line to remove comments from rendered output before comparing it to expected output.
Changes
Solves #78 and #127 (make components optional)