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.

Sign up for GitHub

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

Svelte 5 Migration #161

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Svelte 5 Migration #161

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9fce59e
migration to Svelte 5
evlim Oct 28, 2024
d6f1466
Resolve error in marketing layout
evlim Oct 29, 2024
e100056
Fix type error in change password page
evlim Oct 29, 2024
88cf94f
Resolve type error in create profile page
evlim Oct 29, 2024
682f245
Update packages and format
evlim Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
},
"devDependencies": {
"@sveltejs/adapter-auto": "^3.2.0",
"@sveltejs/kit": "^2.0.0",
"@sveltejs/vite-plugin-svelte": "^3.1.1",
"@sveltejs/kit": "^2.5.27",
"@sveltejs/vite-plugin-svelte": "^4.0.0",
"@tailwindcss/typography": "^0.5.13",
"@types/glob": "^8.1.0",
"@types/html-to-text": "^9.0.4",
Expand All @@ -28,19 +28,19 @@
"daisyui": "^4.7.3",
"eslint": "^8.28.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-svelte": "^2.30.0",
"eslint-plugin-svelte": "^2.45.1",
"fuse.js": "^7.0.0",
"glob": "^10.4.5",
"html-to-text": "^9.0.5",
"jsdom": "^24.1.1",
"postcss": "^8.4.31",
"prettier": "^3.0.3",
"prettier-plugin-svelte": "^3.0.3",
"prettier": "^3.1.0",
"prettier-plugin-svelte": "^3.2.6",
"super-sitemap": "^0.15.1",
"svelte": "^4.2.19",
"svelte-check": "^3.4.3",
"svelte": "^5.0.0",
"svelte-check": "^4.0.0",
Comment on lines +40 to +41
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding Svelte 5 migration testing strategy.

To ensure a smooth transition to Svelte 5:

  1. Add a pre-migration test suite to establish baseline functionality
  2. Document known breaking changes and required updates
  3. Consider adding specific test scripts for migration validation

Add these scripts to the scripts section:

  "scripts": {
+   "test:migration": "vitest run --config vitest.migration.config.js",
+   "validate:svelte5": "svelte-migrate check"
  }

Committable suggestion was skipped due to low confidence.

"tailwindcss": "^3.4.1",
"typescript": "^5.0.0",
"typescript": "^5.5.0",
"vite": "^5.4.8",
"vitest": "^1.0.0"
},
Expand Down
9 changes: 7 additions & 2 deletions src/lib/emails/welcome_email_text.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@
// must be set to true to render email on server
export const ssr = true

// Define all your props here
export let companyName: string = ""

interface Props {
// Define all your props here
companyName?: string;
}

let { companyName = "" }: Props = $props();
</script>

Welcome to {companyName}!
Expand Down
17 changes: 11 additions & 6 deletions src/routes/(admin)/account/(menu)/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@
import { writable } from "svelte/store"
import { setContext } from "svelte"
import { WebsiteName } from "../../../../config"
interface Props {
children?: import("svelte").Snippet
}

let { children }: Props = $props()

const adminSectionStore = writable("")
setContext("adminSection", adminSectionStore)
let adminSection: string
let adminSection: string = $state()
adminSectionStore.subscribe((value) => {
adminSection = value
})
Expand Down Expand Up @@ -47,12 +52,12 @@
</div>
</div>
<div class="container px-6 lg:px-12 py-3 lg:py-6">
<slot />
{@render children?.()}
</div>
</div>

<div class="drawer-side">
<label for="admin-drawer" class="drawer-overlay" />
<label for="admin-drawer" class="drawer-overlay"></label>
<ul
class="menu menu-lg p-4 w-80 min-h-full bg-base-100 lg:border-r text-primary"
>
Expand All @@ -68,7 +73,7 @@
<a
href="/account"
class={adminSection === "home" ? "active" : ""}
on:click={closeDrawer}
onclick={closeDrawer}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider using Svelte's event directive instead of onclick

The change from on:click to onclick might cause issues with Svelte's event handling system. Svelte's on:click directive is preferred as it:

  • Properly handles event delegation
  • Integrates better with Svelte's reactivity system
  • Provides better TypeScript integration

Apply this change to all click handlers:

-onclick={closeDrawer}
+on:click={closeDrawer}

Also applies to: 98-98, 117-117

>
<svg
xmlns="http://www.w3.org/2000/svg"
Expand All @@ -90,7 +95,7 @@
<a
href="/account/billing"
class={adminSection === "billing" ? "active" : ""}
on:click={closeDrawer}
onclick={closeDrawer}
>
<svg
class="h-5 w-5"
Expand All @@ -109,7 +114,7 @@
<a
href="/account/settings"
class={adminSection === "settings" ? "active" : ""}
on:click={closeDrawer}
onclick={closeDrawer}
>
<svg class="h-5 w-5" viewBox="0 0 24 24" stroke="none" fill="none">
<g id="Interface / Settings">
Expand Down
2 changes: 1 addition & 1 deletion src/routes/(admin)/account/(menu)/billing/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("billing")

export let data
let { data } = $props()

let currentPlanId = data.currentPlanId ?? defaultPlanId
let currentPlanName = pricingPlans.find(
Expand Down
2 changes: 1 addition & 1 deletion src/routes/(admin)/account/(menu)/settings/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
let { data } = $props()
let { profile, user } = data
</script>

Expand Down
2 changes: 1 addition & 1 deletion src/routes/(admin)/account/(menu)/settings/change_email/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
let { data } = $props()

let { user } = data
</script>
Expand Down
2 changes: 1 addition & 1 deletion src/routes/(admin)/account/(menu)/settings/change_email_subscription/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script lang="ts">
import SettingsModule from "../settings_module.svelte"
export let data
let { data } = $props()
let { profile } = data
let unsubscribed = profile?.unsubscribed
</script>
Expand Down
19 changes: 13 additions & 6 deletions src/routes/(admin)/account/(menu)/settings/change_password/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
let { data } = $props()
let { user, supabase } = data

// True if definitely has a password, but can be false if they
Expand All @@ -21,11 +21,14 @@
// @ts-expect-error: we ignore because Supabase does not maintain an AMR typedef
let usingOAuth = user?.amr?.find((x) => x.method === "oauth") ? true : false

let sendBtn: HTMLButtonElement
let sentEmail = false
let sendBtn: HTMLButtonElement | undefined = $state()
let sentEmail = $state(false)
let sendForgotPassword = () => {
sendBtn.disabled = true
sendBtn.textContent = "Sending..."
if (sendBtn) {
sendBtn.disabled = true
sendBtn.textContent = "Sending..."
}

let email = user?.email
if (email) {
supabase.auth
Expand All @@ -34,6 +37,10 @@
})
.then((d) => {
sentEmail = d.error ? false : true
if (sendBtn) {
sendBtn.disabled = false
sendBtn.textContent = "Send Forgot Password"
}
})
}
}
Expand Down Expand Up @@ -95,7 +102,7 @@
<button
class="btn btn-outline btn-wide {sentEmail ? 'hidden' : ''}"
bind:this={sendBtn}
on:click={sendForgotPassword}
onclick={sendForgotPassword}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Revert to Svelte's event handling convention

The change from on:click to onclick deviates from Svelte's recommended event handling pattern. Svelte's on:click directive provides better event handling and automatic cleanup.

-onclick={sendForgotPassword}
+on:click={sendForgotPassword}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onclick={sendForgotPassword}
on:click={sendForgotPassword}

>Send Set Password Email
</button>
<div class="success alert alert-success {sentEmail ? '' : 'hidden'}">
Expand Down
2 changes: 1 addition & 1 deletion src/routes/(admin)/account/(menu)/settings/delete_account/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
let { data } = $props()
let { session } = data
</script>

Expand Down
2 changes: 1 addition & 1 deletion src/routes/(admin)/account/(menu)/settings/edit_profile/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
let { data } = $props()

let { profile } = data
</script>
Expand Down
44 changes: 30 additions & 14 deletions src/routes/(admin)/account/(menu)/settings/settings_module.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
}

// Page state
let loading = false
let showSuccess = false
let loading = $state(false)
let showSuccess = $state(false)

type Field = {
inputType?: string // default is "text"
Expand All @@ -21,18 +21,34 @@
maxlength?: number
}

// Module context
export let editable = false
export let dangerous = false
export let title: string = ""
export let message: string = ""
export let fields: Field[]
export let formTarget: string = ""
export let successTitle = "Success"
export let successBody = ""
export let editButtonTitle: string | null = null
export let editLink: string | null = null
export let saveButtonTitle: string = "Save"
interface Props {
// Module context
editable?: boolean
dangerous?: boolean
title?: string
message?: string
fields: Field[]
formTarget?: string
successTitle?: string
successBody?: string
editButtonTitle?: string | null
editLink?: string | null
saveButtonTitle?: string
}

let {
editable = false,
dangerous = false,
title = "",
message = "",
fields,
formTarget = "",
successTitle = "Success",
successBody = "",
editButtonTitle = null,
editLink = null,
saveButtonTitle = "Save",
}: Props = $props()

const handleSubmit: SubmitFunction = () => {
loading = true
Expand Down
12 changes: 8 additions & 4 deletions src/routes/(admin)/account/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
<script lang="ts">
import { run } from "svelte/legacy"

import { invalidate } from "$app/navigation"
import { onMount } from "svelte"

export let data
let { data, children } = $props()

let { supabase, session } = data
$: ({ supabase, session } = data)
let { supabase, session } = $state(data)
run(() => {
;({ supabase, session } = data)
})
Comment on lines +7 to +12
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider simplifying the state synchronization.

The current state synchronization could be more concise by combining the state declaration and synchronization.

-let { supabase, session } = $state(data)
-run(() => {
-  ;({ supabase, session } = data)
-})
+let { supabase, session } = $state(() => data)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let { data, children } = $props()
let { supabase, session } = data
$: ({ supabase, session } = data)
let { supabase, session } = $state(data)
run(() => {
;({ supabase, session } = data)
})
let { data, children } = $props()
let { supabase, session } = $state(() => data)


onMount(() => {
const { data } = supabase.auth.onAuthStateChange((event, _session) => {
Expand All @@ -18,4 +22,4 @@
})
</script>

<slot />
{@render children?.()}
10 changes: 7 additions & 3 deletions src/routes/(admin)/account/create_profile/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
import type { SubmitFunction } from "@sveltejs/kit"
import "../../../../app.css"

export let data
export let form: FormAccountUpdateResult
interface Props {
data: any
form: FormAccountUpdateResult
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving type safety for the Props interface.

While the interface addition is good for structure, the data property using any type reduces type safety. Consider creating a more specific type for the data property based on its expected shape.

 interface Props {
-  data: any
+  data: {
+    user: { email: string }
+    profile?: {
+      full_name?: string
+      company_name?: string
+      website?: string
+    }
+  }
   form: FormAccountUpdateResult
 }

Committable suggestion was skipped due to low confidence.


let { data, form }: Props = $props()

let { user, profile } = data

let loading = false
let loading = $state(false)
Comment on lines +21 to +25
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider optimizing the data destructuring pattern.

While the current implementation works, you could simplify the destructuring pattern to avoid the intermediate step.

- let { data, form }: Props = $props()
- let { user, profile } = data
+ let { data: { user, profile }, form }: Props = $props()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let { data, form }: Props = $props()
let { user, profile } = data
let loading = false
let loading = $state(false)
let { data: { user, profile }, form }: Props = $props()
let loading = $state(false)

let fullName: string = profile?.full_name ?? ""
let companyName: string = profile?.company_name ?? ""
let website: string = profile?.website ?? ""
Expand Down
4 changes: 2 additions & 2 deletions src/routes/(admin)/account/sign_out/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
import { goto } from "$app/navigation"
import { onMount } from "svelte"

export let data
let { data } = $props()

let { supabase } = data
let message = "Signing out...."
let message = $state("Signing out....")

// on mount, sign out
onMount(() => {
Expand Down
17 changes: 12 additions & 5 deletions src/routes/(marketing)/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
<script>
import { WebsiteName } from "./../../config"
import "../../app.css"
/**
* @typedef {Object} Props
* @property {import('svelte').Snippet} [children]
*/

/** @type {Props} */
let { children } = $props()
</script>

<div class="navbar bg-base-100 container mx-auto">
Expand All @@ -19,7 +26,7 @@
<li class="md:mx-2"><a href="/pricing">Pricing</a></li>
<li class="md:mx-2"><a href="/account">Account</a></li>
<li class="md:mx-0">
<a href="/search">
<a href="/search" aria-label="Search">
<svg
fill="#000000"
class="w-6 h-6"
Expand All @@ -34,8 +41,8 @@
</li>
</ul>
<div class="dropdown dropdown-end sm:hidden">
<!-- svelte-ignore a11y-label-has-associated-control -->
<!-- svelte-ignore a11y-no-noninteractive-tabindex -->
<!-- svelte-ignore a11y_label_has_associated_control -->
<!-- svelte-ignore a11y_no_noninteractive_tabindex -->
<label tabindex="0" class="btn btn-ghost btn-circle">
<svg
xmlns="http://www.w3.org/2000/svg"
Expand All @@ -51,7 +58,7 @@
/></svg
>
</label>
<!-- svelte-ignore a11y-no-noninteractive-tabindex -->
<!-- svelte-ignore a11y_no_noninteractive_tabindex -->
<ul
tabindex="0"
class="menu menu-lg dropdown-content mt-3 z-[1] p-2 shadow bg-base-100 rounded-box w-52 font-bold"
Expand All @@ -72,7 +79,7 @@
</div>

<div class="">
<slot />
{@render children?.()}
</div>

<!-- Spacer grows so the footer can be at bottom on short pages -->
Expand Down
Loading
Loading