Skip to content
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

Type checking does not carry across spreading ...$$restProps #1363

Closed
rgossiaux opened this issue Feb 1, 2022 · 5 comments
Closed

Type checking does not carry across spreading ...$$restProps #1363

rgossiaux opened this issue Feb 1, 2022 · 5 comments

Comments

@rgossiaux
Copy link

rgossiaux commented Feb 1, 2022

Describe the bug
If a component A spreads its $$restProps to some child component B, and B has some typed prop foo, then any foo which is passed to A should be checked against the type specified in B. For example, if B specifies that foo is a string, then <A foo={console.log}> should give a type error. Right now it does not.

For that matter, you can also just pass invalid props to A which don't actually exist on A or B, and no error will be thrown.

I can imagine why this could get complicated, but it'd still be really nice if it worked. I ran into this trying to make some TypeScript improvements in svelte-headlessui

To Reproduce
I created a repo for ease of illustration: https://github.com/rgossiaux/svelte-prop-spread-ts

Expected behavior
See repo

Screenshots
N/A

System (please complete the following information):

  • Plugin/Package: svelte-check v2.4.1
@rgossiaux rgossiaux added the bug Something isn't working label Feb 1, 2022
@jasonlyu123 jasonlyu123 removed the bug Something isn't working label Feb 1, 2022
@dummdidumm
Copy link
Member

The component could be used in many different circumstances, so it's not clear statically what $$restProps actually contains. The whole idea of $$restProps is to allow dynamic behavior at runtime, which can't be statically analysed and therefore can't be typed. We won't add whole-app-analysation to check all possible values of $$restProps, therefore closing this.

@rgossiaux
Copy link
Author

The whole idea of $$restProps is to allow dynamic behavior at runtime, which can't be statically analysed and therefore can't be typed.

Hmm not sure I understand. What's an example of this?

I don't see any reason in principle why the example repo I linked could not be typed better. The <Child> component has

    props: {
        name?: string;
    };

The <Parent> component simply passes through all its props to <Child>, so it should be typed the same way, but instead it has:

    props: {
        [x: string]: any;
    };

It doesn't matter what circumstances either of these two components are used in, you only have to analyze the components themselves to type them properly.

@rgossiaux
Copy link
Author

After poking around the svelte2tsx code a little bit, maybe I understand better. You're worried about dynamic access, like $$restProps[someVar], right? So we're talking about dynamic behavior within the component itself.

However going back to:

The whole idea of $$restProps is to allow dynamic behavior at runtime

I'm still not sure that's true. On the balance that's probably true for $$props, but I think the idea of $$restProps is generally to spread it to some child component. In fact I'm not sure there's many other reasons why you would use it; for some dynamic behavior stuff you'd likely just use $$props. So I do think that this problem could be solved statically for the normal cases.

However I would also believe you if you said that this was far too complicated to add (I don't really know) and that I need to just convert all my components over to $$Props to add this typing myself.

@dummdidumm
Copy link
Member

You want the type of $$restProps to be different depending on whether I pass it to component A or component B. But TypeScript does not analyze type control flow this way. The type definitions are top down. You can't pass something to a function and change the value of the parameter depending on where you pass it to, it's the other way around.

@rgossiaux
Copy link
Author

You're right, that makes sense. Thanks for the explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants