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

feat: Migrate Svelte package to Svelte 5 #1049

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: Migrate Svelte package to Svelte 5 #1049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
274632b
Svelte 5 update: updated lib dependencies & components
AlanAcDz Oct 25, 2024
599c80a
Svelte 5 update: use runes in create uploadthing
AlanAcDz Oct 25, 2024
2513c1b
Svelte 5 update: use runes in create dropzone
AlanAcDz Oct 25, 2024
61e1767
Svelte 5 update: bug fixes
AlanAcDz Oct 26, 2024
87ba342
Svelte 5 update: updated example
AlanAcDz Oct 26, 2024
1b20793
Svelte 5 update: updated dependencies
AlanAcDz Nov 12, 2024
7bb86ff
Svelte 5 update: fixed snippets
AlanAcDz Nov 12, 2024
7236c31
Svelte 5 update: updated docs
AlanAcDz Nov 12, 2024
045a86d
Svelte 5 update: sync main
AlanAcDz Nov 12, 2024
b41092e
Merge branch 'main' into svelte-5-update
AlanAcDz Nov 12, 2024
a9f0495
Svelte 5 update: updated package version
AlanAcDz Nov 12, 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
54 changes: 35 additions & 19 deletions docs/src/app/(docs)/getting-started/svelte/page.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ First, generate a typed helper using the `generateSvelteHelpers` function from
<script lang="ts">
import { createUploadThing } from "$lib/utils/uploadthing";

const { startUpload } = createUploadThing("imageUploader", {
const ut = createUploadThing("imageUploader", {
onClientUploadComplete: () => {
alert("Upload Completed");
},
Expand All @@ -237,18 +237,26 @@ First, generate a typed helper using the `generateSvelteHelpers` function from

<input
type="file"
on:change={async (e) => {
onchange={async (e) => {
const file = e.currentTarget.files?.[0];
if (!file) return;

// Do something with files

// Then start the upload
await startUpload([file]);
await ut.startUpload([file]);
}}
/>
```

<Warning>
Previous versions of `@uploadthing/svelte` used stores as a way to pass state
around. New versions use
[runes](https://svelte.dev/docs/svelte/what-are-runes) instead, so things like
destructuring the object returned by `createUploadThing` is not advised any
more as it breaks reactivity.
</Warning>

## Theming Svelte components

Check out our [theming guide](/concepts/theming) to know the basics about
Expand All @@ -257,39 +265,47 @@ Check out our [theming guide](/concepts/theming) to know the basics about
`@uploadthing/svelte`.

However, [content customization](/concepts/theming#content-customization) makes
use of Svelte's named slots and slot props.
use of Svelte's snippets.

### UploadButton content customization

```svelte
<UploadButton {uploader}>
<span slot="button-content" let:state>
{#snippet button(state)}
{state.isUploading ? "Uploading..." : "Pick a file"}
</span>
<span slot="clear-btn" let:state>
{/snippet}
{#snippet clearBtn(state)}
Clear files
</span>
<span slot="allowed-content" let:state>
{/snippet}
{#snippet allowedContent(state)}
You can choose between {state.fileTypes.join(", ")} files
</span>
{/snippet}
</UploadButton>
```

### UploadDropzone content customization

```svelte
<UploadDropzone {uploader}>
<i slot="upload-icon" let:state>
<!-- some custom icon -->
</i>
<span slot="button-content" let:state>
{#snippet uploadIcon(state)}
<i>
<!-- some custom icon -->
</i>
{/snippet}
{#snippet button(state)}
{state.isUploading ? "Uploading..." : "Pick a file"}
</span>
<span slot="label" let:state>
{/snippet}
{#snippet label(state)}
{state.ready ? "Ready to upload" : "Loading..."}
</span>
<span slot="allowed-content" let:state>
{/snippet}
{#snippet allowedContent(state)}
You can choose between {state.fileTypes.join(", ")} files
</span>
{/snippet}
</UploadDropzone>
```

<Warning>
Previous versions of `@uploadthing/svelte` used named slots for content
customization. New versions use
[snippets](https://svelte.dev/docs/svelte/snippet) instead.
</Warning>
Comment on lines +307 to +311
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add migration guidance to the warning

The warning about slots to snippets transition could be more helpful with migration examples:

<Warning>
  Previous versions of `@uploadthing/svelte` used named slots for content
  customization. New versions use
  [snippets](https://svelte.dev/docs/svelte/snippet) instead.
+
+ To migrate your code:
+ ```svelte
+ <!-- Before (slots) -->
+ <UploadButton {uploader}>
+   <svelte:fragment slot="button">
+     Upload
+   </svelte:fragment>
+ </UploadButton>
+
+ <!-- After (snippets) -->
+ <UploadButton {uploader}>
+   {#snippet button(state)}
+     Upload
+   {/snippet}
+ </UploadButton>
+ ```
</Warning>

12 changes: 6 additions & 6 deletions examples/minimal-sveltekit/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
},
"dependencies": {
"@fontsource-variable/inter": "^5.0.17",
"@uploadthing/svelte": "7.1.1",
"@uploadthing/svelte": "7.2.0",
"uploadthing": "7.3.0"
},
"devDependencies": {
"@sveltejs/adapter-auto": "^3.1.1",
"@sveltejs/kit": "^2.5.4",
"@sveltejs/vite-plugin-svelte": "^3.1.0",
"svelte": "^4.2.12",
"svelte-check": "^3.6.7",
"@sveltejs/adapter-auto": "^3.3.1",
"@sveltejs/kit": "^2.8.0",
"@sveltejs/vite-plugin-svelte": "^4.0.0",
"svelte": "^5.1.15",
"svelte-check": "^4.0.7",
Comment on lines +20 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Known Issues with Current Svelte 5 Versions

Several open bug reports exist for Svelte 5.0, indicating potential instability. Thorough testing is recommended before proceeding.

🔗 Analysis chain

Review Svelte 5 ecosystem version compatibility

The updates to Svelte ecosystem packages represent a major version jump:

  • Svelte 5.1.15 (major version upgrade)
  • svelte-check 4.0.7 (major version upgrade)
  • vite-plugin-svelte 4.0.0 (major version upgrade)

These versions align with the Svelte 5 migration objective, but there are some considerations:

  1. Svelte 5 is still in beta/preview
  2. The combination of these specific versions should be tested thoroughly

Consider adding a note in the project documentation about:

  • The experimental nature of Svelte 5
  • Potential breaking changes in future releases
  • Migration steps for users updating from Svelte 4
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility between selected versions

# Check if these specific versions are recommended together
curl -s https://api.github.com/repos/sveltejs/svelte/releases | \
  jq -r '.[] | select(.tag_name | contains("5.1.15")) | .body' | \
  grep -i "compatibility"

# Check for known issues with this version combination
gh api graphql -f query='
{
  repository(owner: "sveltejs", name: "svelte") {
    issues(first: 5, states: OPEN, labels: ["5.0", "bug"]) {
      nodes {
        title
        url
        labels(first: 5) {
          nodes {
            name
          }
        }
      }
    }
  }
}'

Length of output: 1441

"tslib": "^2.6.2",
"typescript": "^5.5.2",
"vite": "^5.4.8"
Expand Down
6 changes: 3 additions & 3 deletions examples/minimal-sveltekit/src/routes/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
},
});

const { startUpload } = createUploadThing("videoAndImage", {
const ut = createUploadThing("videoAndImage", {
/**
* @see https://docs.uploadthing.com/api-reference/react#useuploadthing
*/
Expand All @@ -35,14 +35,14 @@
<UploadDropzone {uploader} />
<input
type="file"
on:change={async (e) => {
onchange={async (e) => {
const file = e.currentTarget.files?.[0];
if (!file) return;

// Do something with files

// Then start the upload
await startUpload([file]);
await ut.startUpload([file]);
}}
/>
</main>
Expand Down
16 changes: 8 additions & 8 deletions packages/svelte/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@uploadthing/svelte",
"version": "7.1.1",
"version": "7.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider a major version bump due to breaking changes

The PR introduces breaking changes (rune usage requirements, snippet props instead of slots), but the version is only bumped from 7.1.1 to 7.2.0. According to semver, breaking changes should trigger a major version bump.

-  "version": "7.2.0",
+  "version": "8.0.0",
📝 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
"version": "7.2.0",
"version": "8.0.0",

"license": "MIT",
"scripts": {
"dev": "vite dev",
Expand Down Expand Up @@ -29,23 +29,23 @@
"!dist/**/*.spec.*"
],
"peerDependencies": {
"svelte": "^4.0.0 || ^5.0.0",
"svelte": "^5.0.0",
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

💡 Codebase verification

Breaking change for Svelte 5 upgrade is not documented in the changelog

Please ensure that the migration to Svelte 5 and the dropping of Svelte 4 support are clearly documented in the changelog or release notes.

🔗 Analysis chain

Verify breaking change documentation for Svelte 5 requirement

The package now requires Svelte 5 and drops support for Svelte 4. This is a breaking change that needs to be clearly documented in the changelog/release notes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if breaking change is documented in changelog

# Look for CHANGELOG.md in the repository
if [[ -f CHANGELOG.md ]]; then
  echo "Checking CHANGELOG.md for Svelte 5 migration notes..."
  rg -i "svelte.*5|breaking.*change" CHANGELOG.md
fi

# Check if there's any migration guide
fd -g "*MIGRATION*.md"

Length of output: 4687

Copy link
Member

Choose a reason for hiding this comment

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

rabbit is correct - this PR would require a major bump to follow semver. are the gains here worth it? my immediate thought is this would limit adoption as I assume not a whole lot of users are using Svelte 5 yet?

I guess uploadthing users are more of the bleeding-edge type of users so they're more likely to upgrade sooner, but still a question worth asking. cc @markflorkowski

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is tough because it also implies that Svelte package is ahead of other frameworks, where functionally it is not.

Is it possible to add support for svelte 5 while maintaining back compat? I assume not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand the concerns, and while I do believe many svelte users have adopted v5, I have no idea how big the adoption might truly be. 🤔 maybe create a new package, something like "svelte-next"? That would keep back compat and make a smoother transition

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I don’t love the idea of introducing a new package for this. I’ll take a look at other packages tomorrow to see how others are handling this. Might be best to just bite the bullet and do the major… really not sure.

"uploadthing": "^7.2.0"
},
"dependencies": {
"@uploadthing/shared": "workspace:*",
"file-selector": "0.6.0"
},
"devDependencies": {
"@sveltejs/adapter-auto": "^3.1.1",
"@sveltejs/kit": "^2.5.4",
"@sveltejs/package": "^2.3.0",
"@sveltejs/vite-plugin-svelte": "^3.1.0",
"@sveltejs/adapter-auto": "^3.3.1",
"@sveltejs/kit": "^2.8.0",
"@sveltejs/package": "^2.3.7",
"@sveltejs/vite-plugin-svelte": "^4.0.0",
"postcss": "8.4.38",
"postcss-load-config": "^5.0.3",
"publint": "^0.2.7",
"svelte": "^4.2.12",
"svelte-check": "^3.6.7",
"svelte": "^5.1.15",
"svelte-check": "^4.0.7",
"tailwindcss": "^3.4.1",
"tslib": "^2.6.2",
"typescript": "^5.5.2",
Expand Down
8 changes: 6 additions & 2 deletions packages/svelte/src/lib/component/Cancel.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
<script lang="ts">
import type { ClassListMerger } from "@uploadthing/shared";

export let cn: ClassListMerger;
export let className: string;
type Props = {
cn: ClassListMerger;
className: string;
};

let { cn, className }: Props = $props();
</script>

<svg
Expand Down
Loading
Loading