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

migrate fails on $$Props #13178

Closed
benmccann opened this issue Sep 9, 2024 · 6 comments · Fixed by #13305
Closed

migrate fails on $$Props #13178

benmccann opened this issue Sep 9, 2024 · 6 comments · Fixed by #13305
Milestone

Comments

@benmccann
Copy link
Member

Describe the bug

The migration tool fails to migrate $$Props, which I didn't know existed until I tried to run it on some real-world projects and it bombed out upon encountering it 😆

A description of $$Props is here: https://www.reddit.com/r/sveltejs/comments/1778zq4/interface_props_mentioned_in_the_svelte_5_post/

migrate lives in this repo:

export function migrate(source) {

Reproduction

https://github.com/immich-app/immich/blob/8cf33690b8ddd8e36bdf5d968c3d5700bfcc2949/web/src/lib/components/shared-components/password-field.svelte#L7

Logs

No response

System Info

latest svelte 5 and unmerged migration tool PR

Severity

annoyance

@dummdidumm
Copy link
Member

This is a tough one to fix. The problem is that the analysis phase errors on $$Props because it thinks this is an identifier. It does think that because it expects types to be stripped out before analyzing the AST (which is very sensible and more robust approach for the rest of the compiler). But for migration we can't strip out the types, because it means we wouldn't be able to properly modify the code afterwards.
The only solution I see right now is to do some kind of quick hack where $$Props is replaced with something like _$$Props so the analysis doesn't throw.

@benmccann
Copy link
Member Author

Hmm, doesn't look like the fix worked. Running npx svelte-migrate@latest svelte-5 in the web directory of https://github.com/immich-app/immich still spits out lots of errors

@benmccann benmccann reopened this Sep 27, 2024
@paoloricciuti
Copy link
Member

@benmccann i just tried locally and the only error i got is this

image

Which unfortunately should be acorn-ts and i don't think we can do much about it and a duplicate of #13179...were you seeing some other errors?

@benmccann
Copy link
Member Author

Yeah, I see a bunch of errors like:

Error while migrating Svelte code
migrate.svelte:45:7 `$$Props` is an illegal variable name. To reference a global variable called `$$Props`, use `globalThis.$$Props` 43: 
44: <script lang="ts">
45:   type $$Props = Props;
                  ^
46: 
47:   export let type: $$Props['type'] = 'button';

I see it says you have a dirty working dir. You didn't happen to have those previously migrated or anything?

It'd be nice to have svelte-migrate print its version when running to make sure we're both running the same version. I tried even with npx svelte-migrate@latest svelte-5 to make sure it was giving me the latest and still get these errors

@paoloricciuti
Copy link
Member

Nope it was dirty because I've installed with pnpm and created the lock file. can you share a component where you have this error? You can even copy paste that into the repl and try migrate there

@benmccann
Copy link
Member Author

wtf. now it worked after trying like 5 times 15 minutes ago. must be some weird npx caching

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 a pull request may close this issue.

3 participants