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

Feat: Svelte 5 Migration #152

Merged
merged 57 commits into from
Nov 30, 2024
Merged

Feat: Svelte 5 Migration #152

merged 57 commits into from
Nov 30, 2024

Conversation

Ahmedhossamdev
Copy link
Member

@Ahmedhossamdev Ahmedhossamdev commented Nov 29, 2024

Fixes: #140, #150

Key Changes:

  • Used $state to define reactive variables instead of let.
  • Replaced $: with $derived or $effect (Runes support)
  • Used $props instead of export let for component properties.
  • Changed event handlers from on:click to onclick for consistency, along with similar event changes.
  • Replaced slots with snippets.
  • Removed deprecated event dispatcher and replaced it by passing the function directly to child components.
  • Renamed some functions for better clarity and naming conventions.
  • Enhanced routeClick with a flyTo transaction for smoother navigation.

For more details, refer to the Svelte v5 Migration Guide.

  • Write a detailed PR description
  • Ensure everything works correctly

…erties and simplify SearchField event binding
@Ahmedhossamdev Ahmedhossamdev changed the title Feat/svelte5-migrate Feat: Svelte 5 Migration Nov 29, 2024
Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

looks great! Everything looks like it's working perfectly, and you've fixed #150, too!

I see one comment from the migration task that needs to be addressed—either by deleting the comment or doing whatever work it wasn't able to do, and then we can merge.

@@ -1,50 +1,31 @@
<!-- @migration-task Error while migrating Svelte code: $$props is used together with named props in a way that cannot be automatically migrated. -->
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 there's some more work to do here.

Copy link
Member Author

@Ahmedhossamdev Ahmedhossamdev Nov 29, 2024

Choose a reason for hiding this comment

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

No, I forgot to remove the comment. It appeared because the migration script failed to remove $$props automatically and it should be done manually, but it has been migrated now to Svelte 5, so there are no errors.

@Ahmedhossamdev
Copy link
Member Author

looks great! Everything looks like it's working perfectly, and you've fixed #150, too!

Haha, I’m glad it got fixed with the migration! I didn’t specifically fix #150, but it’s great to see that everything worked out. 😄 @aaronbrethorst

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

terrific work, @Ahmedhossamdev

@aaronbrethorst aaronbrethorst merged commit 66c5214 into main Nov 30, 2024
3 checks passed
@aaronbrethorst aaronbrethorst deleted the feat/svelte5-migrate branch November 30, 2024 00:32
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.

Upgrade to Svelte 5
2 participants