Skip to content

fix: TypeScript detection fails when <script lang="ts"> is at the bottom - #2914

Closed
abdel-17 wants to merge 4 commits into
sveltejs:masterfrom
abdel-17:fix-script-ts-parsing
Closed

abdel-17 wants to merge 4 commits into
sveltejs:masterfrom
abdel-17:fix-script-ts-parsing

Conversation

@abdel-17

@abdel-17 abdel-17 commented Jan 12, 2026 •

Copy link
Copy Markdown

fixes #2854

In the following example, the svelte language server would fail to detect TS when <script lang="ts"> is at the bottom.

{#snippet foo({props}: {props?: Record<string, unknown>})}{/snippet}

<script lang="ts"></script>

I asked Codex to fix the issue and it did. The LSP no longer fails. I made sure to run the tests and nothing broke. I admit I don't understand the code very well, but it seems to work.

Before:
A screenshot of the example with an error indicating that TS is not detected

After:
A screenshot of the example without the error

@changeset-bot

changeset-bot Bot commented Jan 12, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a91d41c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

const char = html[i];
if (char === '{') {
depth++;
} else if (char === '}' && depth > 0) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not sure if the depth > 0 check in the else if here is correct. It looks wrong.

.substring(moustacheCheckStart, moustacheCheckEnd)
.lastIndexOf('{', offset);

const lastMustacheTagStart = index === -1 ? null : moustacheCheckStart + index;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The reason this only check a substring is due to performance. It can be expensive to repeatedly check lastIndex. So this can't be removed.

@jasonlyu123

Copy link
Copy Markdown
Member

Thank you for the contribution. I am not really sure if this is the proper way to fix this. Checking { count could work in a simple expression, but you'll have a problem if there is a { or } in the string.

To be honest, I'll suggest you write your script tag at the top. You might not have a good experience by putting the script tag at the bottom. It'll be affected by a syntax error during editing way more than if you put it at the top.

@abdel-17

Copy link
Copy Markdown
Author

Yeah I wasn't sure if the code was correct either.

The decision to write script tags at the top isn't mine unfortunately. We have a prettier config that reorders script tags to the bottom. The "solution" so far has been to manually override this for files that cause a problem, but it isn't ideal. Is there a better way to deal with this?

@abdel-17

abdel-17 commented Jan 12, 2026 •

Copy link
Copy Markdown
Author

I reverted the AI's code. I tried implementing a fix by detecting type definition patterns when the parser hits < or >, but it was too complicated. Not sure how else to solve this. Would it break source maps if the parseHtml function placed the script at the top as a preprocessing step?

@paoloricciuti

Copy link
Copy Markdown
Member

I wonder how costly would be to do a single pass parsing of the whole component to find script lang ts beforehand.

@jasonlyu123

jasonlyu123 commented Jan 13, 2026 •

Copy link
Copy Markdown
Member

The reason we use vscode-html-languageservice to parse it here instead of the Svelte compiler is mostly that it is more forgiving of syntax errors, at least in the Svelte 3/4 days when this is written. The result AST is also used in various other places.

One solution I can think of is to use the Svelte compiler as a fallback when no script tag is found.

@paoloricciuti

Copy link
Copy Markdown
Member

From what I understand the problem is not that it doesn't find the script tag but that it start parsing without knowing it has to use TS and that leads to not finding the script tag. So my idea was to do a single pass parsing specifically looking for the script tag only to figure out if we need TS so that we can properly parse everything.

@jasonlyu123

jasonlyu123 commented Jan 13, 2026 •

Copy link
Copy Markdown
Member

From what I understand the problem is not that it doesn't find the script tag but that it start parsing without knowing it has to use TS and that leads to not finding the script tag.

It shouldn't be. The problem is that the vscode-html-languageservice parses the <string as an HTML tag. So the script tag is considered a child of it, not a top-level script tag.

Ideally, we should use a js/ts parse when the html parser encounters < and {. I did try it before, but it is kind of expensive.

@dummdidumm

Copy link
Copy Markdown
Member

Closing in favor of #2921 - thank you!

@dummdidumm dummdidumm closed this Feb 18, 2026
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.

TypeScript is not parsed correctly for snippet arguments

4 participants