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

fix: snippets with TS incorrect transformation #2412

Merged
merged 5 commits into from
Jun 24, 2024

Conversation

paoloricciuti
Copy link
Member

@paoloricciuti paoloricciuti commented Jun 22, 2024

After sveltejs/svelte#12070 the AST for the SnippetBlock has changed (to be more correct).

The problem is that svelte2tsx was using that wrong information for the transformation. This means that currently last version of svelte produces wrong ts code from svelte2tsx.

The problem is that before parameter end was including the typeAnnotation
image
while now it stops where it should (at the parameter)
image

this PR fixes this problem by looking at the end of typeAnnotation if there's one and falling back to the end of the parameter otherwise.

I tried to update a test but despite messing with the expected to make it fail it was still passing so i think i need a bit of guidance over this (sorry).

Also currently there's one failing test for the language server (in the SemanticTokenProvider so i have to check what's that.

Ok by debugging the language server it seems like the type semantic tokens are lost in the snippet...will try to fix

Lol scratch last one it was failing in master too, i just needed a pnpm i

@paoloricciuti
Copy link
Member Author

Ok this should be ready...however ci is failing here and not locally for something seemingly unrelated. Maybe it's a flaky?

@jasonlyu123
Copy link
Member

jasonlyu123 commented Jun 23, 2024

Yes, it's a flaky test. I only saw it once or twice locally so haven't figured out what the problem is 😅.

@dummdidumm dummdidumm merged commit bdfa37a into sveltejs:master Jun 24, 2024
3 checks passed
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 this pull request may close these issues.

3 participants