-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: provide migration function #11334
Conversation
Provides the start of a migration function, exported as `migrate` from `svelte/compiler`, which tries its best to automatically migrate towards runes, render tags (instead of slots) and event attributes (instead of event handlers) The preview REPL was updated with a migrate button so people can try it out in the playground. closes #9239
🦋 Changeset detectedLatest commit: 9dc124b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This is so beautiful 😍😍😍 |
A couple of extra notes maybe:
|
Yeah, I think this might be the best option: <script>
import { run } from 'svelte/legacy';
run(() => {
// ...
});
</script> It'd be nice if we could avoid that in cases like this: $: if (browser) {...} $: if (typeof window !== 'undefined') {...} |
Very small detail but the 'migrate' button should be disabled/greyed out in runes mode |
Why exporting run from legacy? I feel like it could be a good helper in some occasions. |
The migration also converts slots and event handlers, and you could use those in runes mode, too. What then? |
Ah, true. Perhaps it should only be greyed out if we can somehow detect that it wouldn't do anything.
What occasions? |
I think sveltekit had one of those situations right? Where the solution was to duplicate the code once in the script tag and once in the effect? In general everywhere you want to setup something during SSR but also have it react once hydrated. Maybe I'm wrong tho, I hope to use as few effects as possible lol |
Yeah whenever you interact with things you need to prepare imperatively before the dom is SSRd/rendered you need it (in SvelteKit it's initializing stores). Once everything's runes in your code the occasions definitely become less but they're still there. |
let { onclick, ontoggle, 'custom-event-bubble': oncustom_event_bubble } = $props(); | ||
</script> | ||
|
||
<button onclick={(payload) => { |
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.
maybe event
rather than payload
? any idea where the extra space is coming from?
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.
it's because there were multiple on:click handlers and their whitespaces where not removed. I think it's rare enough that this situation even happens to not care (and let prettier handle it; I like your suggestion of running prettier on the result within svelte-migrate)
packages/svelte/tests/migrate/samples/event-handlers/output.svelte
Outdated
Show resolved
Hide resolved
Would it be crazy to run Prettier on the output, if it (and |
@Rich-Harris That feels crazy to me. Nothing else in the Svelte compiler itself is filesystem-y currently, is it? I'd really like to keep these as close to pure functions as we can. |
I think it'd happen in the |
Oh okay, completely different repo. I have much fewer qualms about that, then. |
Provides the start of a migration function, exported as
migrate
fromsvelte/compiler
, which tries its best to automatically migrate towards runes, render tags (instead of slots) and event attributes (instead of event handlers)The preview REPL was updated with a migrate button so people can try it out in the playground. Conveniently-chosen example that works nicely (click on migrate and see it happening)
closes #9239
TODO
$:
that don't get turned into$derived
. Right now I'm just doing$effect
but that's not equivalent because it doesn't run in SSR. Ideas:effect_or_once
(I don't know how to call this) function which basically runs the code immediately on the server and as an effect on the client$effect.pre
run on the server and use that (there's some issue somewhere that requests this because there are some code paths where this may be unavoidable to have an if-server-run-immediately-version)Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint