-
-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Supports union types in props and reactive statement #214
Comments
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? |
change the <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 |
Ah, did not think about that. Thanks! |
The function callback for props won't work lol. Needs to find another way. |
By the way, this wrapping can be used in |
Another problem, this union type props will also have a type of string in the exported prop def because of the same reason |
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 |
export let test: string | number = '' as any; works |
@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 |
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 |
|
playground example |
Then it will be |
Because it can't be |
If you add <!-- 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 |
because svelte compiler compile it to let { test = '' } = $$props so test can't be 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. |
It still can be undefined if you pass a variable that changes to undefined. |
Then it's reactive statement that's possibly |
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
} |
good point! It can differentiate optional props and possibly undefined |
That is what i mean, with the optional props: PatrickG@c5f00f7 And with the Can I create a PR with my changes? |
I think the first one is ok! let a: string | number = true as any; Also, the casting needs to be done through |
Oh, right. 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`
I've not looked into the jsdoc stuff yet |
update: there is another way to do 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; |
I had the re-assignment in my mind too, but wanted to prevent it, because I think it is harder to do^^ |
Yes. I have implemented reassign here but still need some works in cleanup and test. |
#214 * infer type from implicit declarations * test for reactive declaration * add editorconfig for svelte2tsx test files * wrap reative block with arrow function * invalidate control flow type when props is typed and has default * handle object wrapping * cleanup * remove undefined type from invalidated prop * change to cast default to whatever user has defined * change to do typ-assertion in reassign * fix test, cleanup * rollback infer type from implicit declarations for now * (fix) var name interpolation
@jasonlyu123 is everything the issue was about fixed by your PR? (asking for whether or not I can close this) |
Yeah. Thanks |
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.
Describe the solution you'd like
In the
svelte2tsx
, theprops
and reactive statement$: { }
needs to be taken out of the control flow context maybe by wrapping it with a callback like this:Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: