Language-tools: slot props type when reference block scope variable

Created on 2 Jul 2020  路  16Comments  路  Source: sveltejs/language-tools

Describe the bug
when slot props is referencing a block scope variable it would have any type in the consumer

To Reproduce
Steps to reproduce the behavior:

Component.svelte

<script>
    let list = ['']
</script>

{#each list as value}
  <slot props={value} />
{/each}

consumer

<Component let:name></Component>

where name has a type of any

Expected behavior
have correct type

Screenshots
If applicable, add screenshots to help explain your problem.
鍦栫墖
鍦栫墖

System (please complete the following information):

  • OS: Windows
  • IDE: VSCode
  • Plugin/Package: Svelte for VSCode, svelte2tsx

Additional context
got transform to

<></>;function render() { 
 let list = [''];
 ; <>

 {(list).map((value) => <>

 <slot props={value} />
 </>)}</> return { props: {}, slots: {default: {props:value}} }} export default class { $$prop_def = __sveltets_partial(render().props) $$slot_def = render().slots }
Fixed bug

Most helpful comment

Maybe this could really work in a generic way:

  • If you enter a #each, do (__svelte_ts_unwrappAttr(list))
  • If you enter a #await, do the same but with __svelte_ts_unwrapPromiseLike(promise)
  • If they use destructuring, wrap it with that immediately invoked function @jasonlyu123 proposed
  • Do this recursivly if needed

All 16 comments

The blocks where the variable is declared probably has to return the props definition. And it has to pass through multiple bocks. like this:

declare function unwrap<T>(value: Promise<T>| T[]): T
const __$$_default_0_prop = unwrap((list).map((value) =>{ <>

 <slot props={value} />
</>; return { props: value }}));<></>; return { props: {}, slots: { default: { ...__$$_default_0_prop }}}

But the biggest problem is it needs to be moved up until it's not nested in an element or component so that we could event declare any variable.

And we also need to check if the expression references another slot props. The whole process it's so tedious I think it's better off just don't support it and provide a way to let the user manually defined its type.

Maybe something like

interface ComponentDef {
   props?: ... // optional
   slots?: ... // optional
}

That way you could define props explicitely while at the same time define connections between props and slots like wanted in #273 :

interface ComponentDef<T>{
   props: { items: T[]; };
   slots: { item: T };
}

svelte2tsx would check if an interface definition called ComponentDef exists and if yes would use that instead of constructing its own props/slots definition.

One thing that's left: How to use that generic type inside the rest of the component?

interface ComponentDef<T>{
   props: { items: T[]; };
   slots: { item: T };
}

export let items: T[]; // <<-- ??? should that be possible?

The blocks where the variable is declared probably has to return the props definition. And it has to pass through multiple bocks. like this:

declare function unwrap<T>(value: Promise<T>| T[]): T
const __$$_default_0_prop = unwrap((list).map((value) =>{ <>

 <slot props={value} />
</>; return { props: value }}));<></>; return { props: {}, slots: { default: { ...__$$_default_0_prop }}}

But the biggest problem is it needs to be moved up until it's not nested in an element or component so that we could event declare any variable.

Why do allow Promise<T>?
Im not aware that {#each promise as value} is a thing.

Wouldn't that work?

declare function unwrap<T>(value:  T[]): T
function render() {
    let list = ['', 123];

    <>
        {(list).map((item) => {
            return <>
                <slot item={item} />
            </>;
        })}
    </>;

    return {
        props: {},
        slots: {
            default: { item: unwrap(list) }
        }
    };
}

If i hover over unwrap(list) vscode shows the type string | number.

@dummdidumm then you have to specify the prop types two times? 馃

I know, but how else to define the relationship between them?

We could also reserve interface names ComponentProps and ComponentSlots, so if there is no relationship to be modeled between slots and props, you can just define ComponentSlots - no duplicates.

I was thinking about something like that:

declare function unwrap<T>(value:  T[]): T
function render<T>() {
    let list: T[];

    <>
        {(list).map((item) => {
            return <>
                <slot item={item} />
            </>;
        })}
    </>;

    return {
        props: {},
        slots: {
            default: { item: unwrap(list) }
        }
    };
}

But then, you would have to call the render function with the type from the thing you pass to the component.
I have not found where this is called yet. Is it in svelte-check? Or has the render function to stay like it is, because of tsx?

It's called nowhere and that is part of the problem.

Edit: maybe it's called, indirectly through the jsx syntax but I don't know how exactly.

I think we can still provide the way to manually type components even if @PatrickG's solution is easier to implement. It can serve as a fallback for edge-case, bug, or even manually type the event.

interface ComponentEvent {
    on(event: 'click', handler: (e: MouseEvent) => any): void
    on(event: 'some-event', handler: (e: CustomEvent<number>) => void): void
}

If we go with the interface definition solution this is quite a change. Do others have opinions on this?

As for Patrick's solution, I think we'll need to track the variable scope when walking the AST. Whenever we found the slot props referencing a block scope variable we'll have to add the variable and the variable's initializer to a Map. And then in an upper scope. check if the variable initializer and their initializers reference a block scope variable.

for example:

<script>
    export let keys = [];
    export let items = [];
</script>

<table>
{#each items as item}
    <tr>
        {#each keys as key}
            <slot value={item[key]}></slot>
        {/each}
    </tr>
{/each}
</table>
  1. keys each block
    the key would be key and the value would be unwrap(keys)
  2. items each block
    the key would be item and the value would be unwrap(items)
<script>
    export let items = [ [ ] ];
</script>

<table>
{#each items as item}
    <tr>
        {#each item as innerItem}
            <slot value={innerItem}></slot>
        {/each}
    </tr>
{/each}
</table>
  1. item each block
    the key would be innerItem and the value would be unwrap(innerItem)
  2. items each block
    the key would be item and the value would be unwrap(items), unwrap(innerItem) became unwrap(unwrap(items))

There are also type narrowing cases which would need to be considered:

<script lang="ts">
   export let bla: string | null;
</script>
{#if bla !== null}
   <slot value={bla}></slot> // <--- bla is now of type string
{/if}

---> I think these cases are just too hard to infer dynamically for us / it takes too much work for too little benefit. I think letting the user explicitely type slots/props in this case is better.

Edit: Just crossed my mind: If we go with the interface-approach, how can we make sure that the interface is correctly implemented? So for example if someone types interface ComponentSlots { slot1: number } but does <slot slot1={'a string'}></slot>, how do we catch that type error?

We don't need a Map of the variables itself, just the type of the variable at the time it is used in the <slot />.

I found that https://www.typescriptlang.org/docs/handbook/jsx.html#attribute-type-checking - maybe that could be helpful.

By the time of walking through Htmlx AST, there's no information on the type of a variable. Even if it's a typescript AST there's no type information only the type node associate with it.

A crazy way to transform so that we don't have to care about destructuring

function render() {
    let list = [];

    <>
        {(list).map(({ a }) => {
            return <>
                <slot props={a} />
            </>;
        })}
    </>;

    return {
        props: {},
        slots: {
            default: { props: (({ a }) => a)(__sveltets_unwrapArr(list)) }
        }
    };
}

Maybe this could really work in a generic way:

  • If you enter a #each, do (__svelte_ts_unwrappAttr(list))
  • If you enter a #await, do the same but with __svelte_ts_unwrapPromiseLike(promise)
  • If they use destructuring, wrap it with that immediately invoked function @jasonlyu123 proposed
  • Do this recursivly if needed
Was this page helpful?
0 / 5 - 0 ratings

Related issues

dummdidumm picture dummdidumm  路  24Comments

mgholam picture mgholam  路  46Comments

benmccann picture benmccann  路  14Comments

limitlessloop picture limitlessloop  路  22Comments

martonlederer picture martonlederer  路  14Comments