feat: support migration of single assignment labeled statements #13461
Conversation
🦋 Changeset detectedLatest commit: 8ba0012 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 |
| update_idx === -1 && | ||
| (declaration_idx !== -1 || | ||
| (labeled_idx !== -1 && assignment_idx !== -1) || | ||
| (labeled_idx === -1 && assignment_idx === -1)) |
There was a problem hiding this comment.
This might be a bit simpler if you're not searching for the index but rather the object directly. Not as much -1, and no reference.path[idx] above
There was a problem hiding this comment.
Yeah i usually go with indices just because this just stores a reference to a number instead of an object but that's silly in this case.
| /** @type {number} */ (labeled_to_remove.start), | ||
| /** @type {number} */ (labeled_to_remove.end) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Is it necessary to do this up here? What speaks against doing inside the VariableDeclaration visitor?
There was a problem hiding this comment.
Uh actually this was initially needed because i was converting even blocks with multiple assignments but that's no buono so i reverted that change and forgot that now i can do it in VariableDeclaration. Fixing now.
| $: almost_infinity = count * 128; | ||
|
|
||
| let should_be_state; | ||
| $: should_be_state = 42; |
There was a problem hiding this comment.
I also saw things like
$: should_also_be_state = 42;without a preceeding let with the same name in the wild. Would be good to handle that aswell. and transform that into let should_also_be_state = $state(42)
There was a problem hiding this comment.
I planned to do that as well but since that is not a variable declaration i should've repeated a lot of code from here...i planned to do this as a separate PR to not complicate things too much in this. Should i go for this in this PR too?
There was a problem hiding this comment.
Scratch that...it was different. Pushing now.
Svelte 5 rewrite
I took a shot at closing #13460
Also closes #13459
It seems to work fine but i don't know if i'm missing some weird use case i was not able to think about.
Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (
main).If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is
svelte-4and notmain.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.Tests and linting
pnpm testand lint the project withpnpm lint