Skip to content

Commit

Permalink
feat: better type checking for bindings in Svelte 5 (#2477)
Browse files Browse the repository at this point in the history
#1392

This adds enhanced type checking for bindings in Svelte 5: We're not only passing the variable in, we're also assigning the value of the component property back to the variable. That way, we can catch type errors like the child binding having a wider type as the input we give it.
It's done for Svelte 5 only because it's a) easier there b) doesn't break as much code (people who upgrade can then also upgrade the types, or use type assertions in the template, which is only possible in Svelte 5).
There's one limitation: Because of how the transformation works, we cannot infer generics. In other words, we will not catch type errors for bindings that rely on a generic type. The combination of generics + bindings is probably rare enough that this is okay, and we can revisit this later to try to find a way to make it work, if it comes up.
Also note that this does not affect DOM element bindings like <input bind:value={...} />, this is only about component bindings.
  • Loading branch information
dummdidumm authored Aug 28, 2024
1 parent ec5fef4 commit 8c080cf
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,16 @@ export class RenameProviderImpl implements RenameProvider {
const mappedLocations = await Promise.all(
renameLocations.map(async (loc) => {
const snapshot = await snapshots.retrieve(loc.fileName);
const text = snapshot.getFullText();
const end = loc.textSpan.start + loc.textSpan.length;

if (!isTextSpanInGeneratedCode(snapshot.getFullText(), loc.textSpan)) {
if (
!(snapshot instanceof SvelteDocumentSnapshot) ||
(!isTextSpanInGeneratedCode(text, loc.textSpan) &&
// prevent generated code for bindings from being renamed
// (it's not inside a generate comment because diagnostics should show up)
text.slice(end + 3, end + 27) !== '__sveltets_binding_value')
) {
return {
...loc,
range: this.mapRangeToOriginal(snapshot, loc.textSpan),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script lang="ts">
export let legacy1: string = '';
export let legacy2: number | string = '';
</script>

{legacy1}
{legacy2}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script lang="ts">
let { runes1 = $bindable(), runes2 = $bindable() }: { runes1: string, runes2: string | number } = $props();
</script>

{runes1}
{runes2}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script lang="ts" generics="T">
let { foo, bar = $bindable() }: { foo: T, bar?: T } = $props();
</script>

{foo}
{bar}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[
{
"code": 2322,
"message": "Type 'string | number' is not assignable to type 'string'.\n Type 'number' is not assignable to type 'string'.",
"range": {
"end": {
"character": 28,
"line": 17
},
"start": {
"character": 28,
"line": 17
}
},
"severity": 1,
"source": "ts",
"tags": []
},
{
"code": 2322,
"message": "Type 'string | number' is not assignable to type 'string'.\n Type 'number' is not assignable to type 'string'.",
"range": {
"end": {
"character": 45,
"line": 18
},
"start": {
"character": 45,
"line": 18
}
},
"severity": 1,
"source": "ts",
"tags": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<script lang="ts">
import Legacy from './Legacy.svelte';
import Runes from './Runes.svelte';
import RunesGeneric from './RunesGeneric.svelte';
let legacy = '';
let runes = '';
let foo: string | number;
</script>

<!-- ok -->
<Legacy bind:legacy1={legacy} />
<Runes bind:runes1={runes} bind:runes2={(runes as any)} />
<!-- bail on generics -->
<RunesGeneric {foo} bind:bar={runes} />

<!-- error in Svelte 5 -->
<Legacy bind:legacy2={legacy} />
<Runes bind:runes1={runes} bind:runes2={runes} />
12 changes: 6 additions & 6 deletions packages/svelte2tsx/repl/index.svelte
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<script>
export let value;
<script lang="ts">
import MyComponent from './ComponentB.svelte'
let value: Date
$: console.log('value:', value)
</script>

{#if value}
<input bind:value on:change />
{/if}

<MyComponent bind:value />
6 changes: 6 additions & 0 deletions packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ export function handleBinding(
if (isSvelte5Plus && element instanceof InlineComponent) {
// To check if property is actually bindable
element.appendToStartEnd([`${element.name}.$$bindings = '${attr.name}';`]);
// To check if the binding is also assigned to the variable (only works when there's no assertion, we can't transform that)
if (!isTypescriptNode(attr.expression)) {
element.appendToStartEnd([
`${expressionStr} = __sveltets_binding_value(${element.originalName}, '${attr.name}');`
]);
}
}

if (element instanceof Element) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export class InlineComponent {
private startTagEnd: number;
private isSelfclosing: boolean;
public child?: any;
public originalName = this.node.name;

// Add const $$xxx = ... only if the variable name is actually used
// in order to prevent "$$xxx is defined but never used" TS hints
Expand Down
13 changes: 13 additions & 0 deletions packages/svelte2tsx/svelte-shims-v4.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,16 @@ declare function __sveltets_2_isomorphic_component<
declare function __sveltets_2_isomorphic_component_slots<
Props extends Record<string, any>, Events extends Record<string, any>, Slots extends Record<string, any>, Exports extends Record<string, any>, Bindings extends string
>(klass: {props: Props, events: Events, slots: Slots, exports?: Exports, bindings?: Bindings }): __sveltets_2_IsomorphicComponent<__sveltets_2_PropsWithChildren<Props, Slots>, Events, Slots, Exports, Bindings>;

type __sveltets_NonUndefined<T> = T extends undefined ? never : T;

declare function __sveltets_binding_value<
// @ts-ignore this is only used for Svelte 5, which knows about the Component type
Comp extends typeof import('svelte').Component<any>,
Key extends string
>(comp: Comp, key: Key): Key extends keyof import('svelte').ComponentProps<Comp> ?
// bail on unknown because it hints at a generic type which we can't properly resolve here
// remove undefined because optional properties have it, and would result in false positives
unknown extends import('svelte').ComponentProps<Comp>[Key] ? any : __sveltets_NonUndefined<import('svelte').ComponentProps<Comp>[Key]> : any;
// Overload to ensure typings that only use old SvelteComponent class or something invalid are gracefully handled
declare function __sveltets_binding_value(comp: any, key: string): any

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 8c080cf

Please sign in to comment.