(frontend): Incremental migration to svelte 5#2755
Conversation
|
@Keavon I've enabled runes mode for the whole project with the last commit, but still testing it. Let me know if you find anything that's breaking |
|
Thanks. Please keep testing it extensively and do any polishing you find 'til I get the chance to code review this. There are a few PRs in front of yours in the priority queue so it might be a couple of days, but I'll try my best to keep it from taking too long. |
316a737 to
532e913
Compare
|
I'm done with my bug fixes ✌️. Let me know if anything else is breaking as I'm not well aware of the UI hot paths and user workflow |
|
Could you please sync this with master? |
|
Synced! |
|
Once again please, sorry for the trouble! |
No worries, have synced it again |
|
Could you please update it again? I think I'll be able to get to it today since it is finally roughly next in my queue. |
ec51271 to
e025103
Compare
|
Synced! I noticed backend behaviour has changed? Undo doesn't work like before. I tried it on |
|
Thanks! What is the |
|
This is the
Correct me if I'm wrong, but editor_VS_dev_UNDO_behaviour.mp4 |
|
Yes, dev deploys master. |
|
!build |
|
|
What was the reason for needing to rename |
|
Please run |
|
Runes linting testThe following code snippet should've triggered svelte/infinite-reactive-loop linter error on line 15 but it didn't.
Going by above, the lint rules for svelte 5 might not work extensively in all cases, and hence caution must be exercised to not be all reliant on linter for svelte 5 logic Conclusion
Installed: |
|
Bug:
|
This breaks it :( My theory was wrong, I need the widget state to be reactive to make it work with deeply nested bind's located at root level components and layers above. We need to create reactive state and the |
Fixed this |
|
Ok, nice work so far. Please keep me updated on the latest status. |
|
I'm gonna do a final round of testing by tonight after syncing with master. If I don't find any issues, then I'll reopen from draft again for review |
|
I'm done with my testing, with d24c0b6
Aside from last bug, no new bugs have been found and therefore I'm re-opening for a review ✌️ |
|
@Keavon can we build and push this for testing? |
|
!build |
|
|
There's one task remaining which is adding the Aside from that, I feel confident with the build. I'd prefer a merge this Friday or Saturday, as I'd be available to provide with any fixes if needed anytime after that |
|
Would you be interested in submitting a separate PR which upgrades Graphite to Svelte 5 without also migrating to runes? This was far too large to merge in one fell swoop and it needs to be reviewed in incremental steps. The first would be to get it upgrading to Svelte 5 and successfully compiling without changing any code except that which is necessary to get it up and running. After that, we could explore an incremental process of migrating several files at a time to runes until all are completed. |

Migration Plan step by step
dynamicCompileOptionsforvite-plugin-svelte, and add files in the runes whitelist (runesGlobs)svelte/legacydepracation warnings (AvoidingcreateBubblerover here)runescompatible, start chiping away the remainingsvelte/legacycompat bindings.git-blame-ignore-revsfor a cleaner git blameFollowing the svelte 5 migration guide
Known Issues
patchWidgetLayoutdoesn't update deeply nested properties reactively - 3e572c9autofixed when rebased onto masterGraph.svelte(Node Graph) is invisible on load, displays after clicking on screen - 055b6f3NodeCatalog.sveltereactive bindings messed up for sub-menus and node creation - 461aa77FieldInputdoes not reset local component state when hit withEscape- d24c0b6