Language-tools: Supports union types in props and reactive statement

[jasonlyu123]

Is your feature request related to a problem? Please describe.
The following code will have a typescript error due to typescript's control flow analysis.
But this is very useful when it comes to a select-wrapper component.

<script lang="typescript">
  export let value: string | number = '';

  if (typeof value === 'number') {
    console.log(value.toFixed())
  }

</script>

<select bind:value={value}></select>

Describe the solution you'd like
In the svelte2tsx, the props and reactive statement $: { } needs to be taken out of the control flow context maybe by wrapping it with a callback like this:

declare function __sveltets_invalidate<T>(getValue: () => T): T

/*export*/ let value: string | number = __sveltets_invalidate(() => '')

let b: 'A' | 'B' = 'A';

() => {
$: {
    console.log(b)
}
}

Additional context
Add any other context or screenshots about the feature request here.

[dummdidumm]

Good idea with the invalidate-wrapper around props. Maybe we don't even need to add another wrapper to reactive statements then? Or are there any cases where this will throw control flow errors?

[jasonlyu123]

change the export let to let and wrap the statement in $: {} will have the same error

<script lang="typescript">
  let value: string | number = '';

  $: {
    if (typeof value === 'number') {
      console.log(value.toFixed())
    }
  }
</script>

if value got re-assign by event handler. the type of it might chages

[dummdidumm]

Ah, did not think about that. Thanks!

[jasonlyu123]

The function callback for props won't work lol. Needs to find another way.

[jasonlyu123]

By the way, this wrapping can be used in $: variable if we change the first $: label to let like we discussed earlier in the overall strategy thread. Because its type is depending on another let. So it would works.

[jasonlyu123]

Another problem, this union type props will also have a type of string in the exported prop def because of the same reason

[jasonlyu123]

export let someProps : string | number = ''

To

declare function __sveltets__invalidateWithDefault<T>(getValue: () => T | undefined): T;

let someProps: string | number = ''
someProps = __sveltets__invalidateWithDefault(() => someProps);

This works, but kind of hacky

[PatrickG]

export let test: string | number = '' as any;

works

[jasonlyu123]

@PatrickG's solution is better. but both approach would make the props has an undefined type.

export let test: string | number | undefined = '' as any;
test // test is shouldn't be undefined here

[dummdidumm]

Can you get the types from the typescript AST somehow and use that to construct it? Or use some kind of AST traversal/Regex to get the type definition (everything after : and before =) and then put it behind it: export let test: string | number = '' as string | number.

[PatrickG]

@PatrickG's solution is better. but both approach would make the props has an undefined type.

export let test: string | number | undefined = '' as any;
test // test is shouldn't be undefined here

for me test is string | number. Not undefined.

[jasonlyu123]

playground example
It'll be undefined if strictNullChecks is enabled (strict: true also enable it)

[PatrickG]

playground example
It'll be undefined if strictNullChecks is enabled (strict: true also enable it)

Then it will be string | number | undefined. Where is the problem?

[jasonlyu123]

Because it can't be undefined when your default value is not undefined. I want to eliminate that undefined type so that users don't have to do an unnecessary null check to suppress the undefined error.

[PatrickG]

If you add | undefined to the type, of course it can be undefined and you should check that it is not undefined when you try to invoke a method.

<!-- Component.svelte -->
<script lang="ts">
  export let test: string | undefined = '' as any; // `string | undefined` means it can be undefined.

  test.toUpperCase(); // not safe - because `test` can be `undefined`

  test?.toUpperCase(); // safe
</script>

<!-- Other component -->
<script>
  import Component from './Component.svelte';
</script>

<Componen test={undefined} />

The only Problem I see currently, is that the svelte extension should make the prop optional if it has a initial value.

Instead of checking for undefined in the type here, I think we can use VariableDeclaration.initializer here

[jasonlyu123]

because svelte compiler compile it to

let { test = '' } = $$props

so test can't be undefined even if you explicitly pass undefined as props. It can only be undefined if you re-assign it to undefined later.

Maybe it would be too complex or impossible to do what I intend to do. And It might be just an edge case that few people would use.

[PatrickG]

It still can be undefined if you pass a variable that changes to undefined.
https://svelte.dev/repl/hello-world?version=3.23.2

[jasonlyu123]

Then it's reactive statement that's possibly undefined? Maybe we should just do reactive and require user to explicitly cast the type of default value.

[jasonlyu123]

export let a: string | number | undefined = '' as string | number

Casting can work. but in js it won't have undefined type in a function

    /**@type { string | number | undefined }*/
    let name = /**@type {string | number} */ ("world"),
        world = '';

    name

    function a() {
        name
    }

js playground example

[jasonlyu123]

Instead of checking for undefined in the type here, I think we can use VariableDeclaration.initializer here

good point! It can differentiate optional props and possibly undefined

[PatrickG]

That is what i mean, with the optional props: https://github.com/PatrickG/language-tools/commit/c5f00f75df74c8e3dcfc9842dad4256b9d4f29f2

And with the as any after the initalizer, vscode no longer thinks export let test: string | number = ''; is a string. https://github.com/PatrickG/language-tools/commit/7f630bbf7d66a037590384d3e2a7f8b7314b622c

Can I create a PR with my changes?

[jasonlyu123]

I think the first one is ok!
Another cons of as any is that It would not get type-check. So maybe it needs to be cast to whatever user has defined.

let a: string | number = true as any;

Also, the casting needs to be done through jsdoc in js. I have tried that today.
About the undefined problem, I give up on it. Since we can't found a generic function that can do the trick. We could only manually check if its initializer is undefined but not if a variable passed in as the initializer is undefined. The false negatives seem to be more important.

[PatrickG]

I think the first one is ok!
Another cons of as any is that It would not get type-check. So maybe it needs to be cast to whatever user has defined.

let a: string | number = true as any;

Oh, right.
What about this:

declare function __sveltets_initialize<T>(value: T): any;
let test: string | number = __sveltets_initialize<string | number>('');
test; // string | number

let test: string | number = __sveltets_initialize<string | number>(true); // error because `true` is not `string` or `number`

Also, the casting needs to be done through jsdoc in js. I have tried that today.

I've not looked into the jsdoc stuff yet

[jasonlyu123]

update: there is another way to do as any

export let a: number | string = '1',
  b = a;

to

declare function __sveltets_any(dummy: any): any;

let a: number | string = '1'
a = __sveltets_any(a);
let b = a;

No type-assertion needed and the initializer got type-check. So that this would show typing error:

export let a: ISomeInterface = {};
export let b: string | number = true;

[PatrickG]

I had the re-assignment in my mind too, but wanted to prevent it, because I think it is harder to do^^
Do you implement this?

[jasonlyu123]

Yes. I have implemented reassign here but still need some works in cleanup and test.
Another one is focusing on $: variable

[dummdidumm]

@jasonlyu123 is everything the issue was about fixed by your PR? (asking for whether or not I can close this)

[jasonlyu123]

Yeah. Thanks