sveltejs/language-tools

Semantic token highlights entire $props() type annotation

max-got opened this issue · 10 comments

Describe the bug

Just a quick little syntax highlighting bug in a script tag:
Template literals aren't properly highlighted in $props() type declaration:

initial_svelte_vs

As shown above, can be worked around with a separate type

Reproduction

<script lang="ts">
	type Icon = `i-ph-${string}`;
	let { icon, iconType }: { icon: Icon; iconType: `i-ph-${string}` } = $props();
</script>

Expected behaviour

fixed_svelte_vs

System Info

  • IDE: VSCode
  • Svelte for VS Code v108.4.1

Which package is the issue about?

Svelte for VS Code extension

Additional Information, eg. Screenshots

No response

So i've investigated this a bit and the problem is that svelte2tsx generate this code for a component with $props like this

<script lang="ts">
    const {foo}:{foo: string} = $props();
</script>

{foo}
///<reference types="svelte" />
;function render() {
;type $$ComponentProps = {foo: string};
    const {foo}:$$ComponentProps = $props();
;
async () => {

foo;
};
return { props: {} as any as $$ComponentProps, slots: {}, events: {} }}

export default class extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_partial(__sveltets_2_with_any_event(render()))) {
    constructor(options = __sveltets_2_runes_constructor(__sveltets_2_partial(__sveltets_2_with_any_event(render())))) { super(options); }
    $$bindings = __sveltets_$$bindings('');
}

the key point is that instead of having the inline type this line get's converted to

const {foo}:$$ComponentProps = $props();

when the language server map the tokens of this code to the original position $$ComponentProps get's correctly identified as a type alias (infact in the code looks like a single type alias).

This is the code responsible for the generation of that part

} else {
	// Create a virtual type alias for the unnamed generic and reuse it for the props return type
	// so that rename, find references etc works seamlessly across components
	this.$props.type = '$$ComponentProps';
	preprendStr(
		this.str,
		generic_arg.pos + this.astOffset,
		`;type ${this.$props.type} = `
	);
	this.str.appendLeft(generic_arg.end + this.astOffset, ';');
	this.str.move(
		generic_arg.pos + this.astOffset,
		generic_arg.end + this.astOffset,
		node.parent.pos + this.astOffset
	);
	this.str.appendRight(generic_arg.end + this.astOffset, this.$props.type);
}

and just by doing this

} else {
	// Create a virtual type alias for the unnamed generic and reuse it for the props return type
	// so that rename, find references etc works seamlessly across components
	this.$props.type = '$$ComponentProps';
	preprendStr(
		this.str,
		generic_arg.pos + this.astOffset,
		`;type ${this.$props.type} = `
	);
	this.str.appendLeft(generic_arg.end + this.astOffset, ';');
	this.str.move(
		generic_arg.pos + this.astOffset,
		generic_arg.end + this.astOffset,
		node.parent.pos + this.astOffset
	);
-	this.str.appendRight(generic_arg.end + this.astOffset, this.$props.type);
+	this.str.appendRight(generic_arg.end + this.astOffset, generic);
}

the problem is indeed fixed.

However this comment made me think that this might not be the right solution. I tried debugging the vscode extension and the rename seems to work across file but maybe i'm missing something?

I can open the PR with this fix but i think this will need to update a bunch of tests and again i feel like i'm missing something. Maybe there's a way to map the original position to the actual type instead of the alias?

EDIT: found the PR that introduced the type alias #2255 . I don't know if there's a way to direct the language server to the correct line for the tokens of the props tho. Will try to investigate further.

The problem is indeed because of the $$ComponentProps alias. You could probably surround the this.$props.type with the ignore-comment and check if another feature is affected. I can't think of a situation but if you find one we can consider another possible fix.

I vaguely remember there being situations where the intellisense features didn't work properly if not extracting it into an interface. Maybe not all of it is covered with tests yet so it's best to test find references, go to definition, rename etc out manually before and after (and if there's a difference possibly add a regression test)

I vaguely remember there being situations where the intellisense features didn't work properly if not extracting it into an interface. Maybe not all of it is covered with tests yet so it's best to test find references, go to definition, rename etc out manually before and after (and if there's a difference possibly add a regression test)

The PR the introduced it was because of find reference and tbf it make sense since without the type alias those are just two different types with the same shape.

I'll try again but I suspect the issue is there.

The problem is indeed because of the $$ComponentProps alias. You could probably surround the this.$props.type with the ignore-comment and check if another feature is affected. I can't think of a situation but if you find one we can consider another possible fix.

Which of the this.$props.type are you referring to? Also from what I've seen it seems to actual map correctly to the position of the type but the type of token returned from typescript is interface.

Which of the this.$props.type are you referring to?

this one in the code you referenced:

this.str.appendRight(generic_arg.end + this.astOffset, this.$props.type)

from what I've seen it seems to actual map correctly to the position of the type but the type of token returned from typescript is interface

I am not sure where you saw it being interface but it probably doesn't matter much. The problem is that the token doesn't actually exist in the source file so filtering it out should fix it.

Great it seems to work...opening a pr right now.

Just for my learning and understanding:

The problem is that the token doesn't actually exist in the source file so filtering it out should fix it.

What happens when you filter it out? Like where does it get's the actual semantic tokens?

The actual literal type is moved but can still mapped back to the original position. So the semantic tokens of the original literal type still exist. The $$ComponentProps one is generated and originally overlaps with the original type tokens. The whole range of the literal is marked as type because the generated $$ComponentProps is mapped to there

{ icon: Icon; iconType: `i-ph-${string}` }

but within the range, there is also icon and iconType which is property, and Icon is type. VSCode doesn't support overlap so the inner ones probably got filtered out.

The actual literal type is moved but can still mapped back to the original position. So the semantic tokens of the original literal type still exist. The $$ComponentProps one is generated and originally overlaps with the original type tokens. The whole range of the literal is marked as type because the generated $$ComponentProps is mapped to there

{ icon: Icon; iconType: `i-ph-${string}` }

but within the range, there is also icon and iconType which is property, and Icon is type. VSCode doesn't support overlap so the inner ones probably got filtered out.

Ohhhhh yeah this makes total sense