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

The flow components are not typed welll #1

Closed
louiss0 opened this issue Jan 29, 2023 · 4 comments
Closed

The flow components are not typed welll #1

louiss0 opened this issue Jan 29, 2023 · 4 comments

Comments

@louiss0
Copy link
Contributor

louiss0 commented Jan 29, 2023

The <For> and the <Case> components are not typed well at all.

  • The for component is supposed to accept an iterable the the children are supposed to be typed based on the iterable.
  • The <For> is supposed to just have one key pick one each= or of= if you allow multiple random keys then there will be random types coming from the children
  • The case component is supposed to be typed like this.
type FilledCase= {
    of:string | number | bigint | boolean | symbol | null; 
    default?:never
    }
type DefaultCase={
    default:true;
    of?:never 
    
    }
type Case =  FilledCase |  DefaultCase

This typing makes sure that the user only types of or the default on set to true

The for component is supposed to be typed like this

interface Loopable {
	forEach(cb:(...args:Array<unknown>)=> unknown, thisArgs:unknown):void
}


interface ForComp<T extends Loopable> {
	of: T 
	children: Parameters<T["forEach"]>[0]
}

I know this solution forces the user to only pass in the data structure that they intend to use and not use anything that would come from the result of a method like .entries() but I don't think It's worth trying to use an overload to solve this problem. THe best way to solve this problem is to find out how for of is typed. This solution allows developers to figure out the rest.

@jonathantneal
Copy link
Collaborator

jonathantneal commented Jan 29, 2023

Hi, @louiss0,

Thanks for letting me know about this. I’ll put these suggestions into a PR. Would you be willing to review that PR? Or would you prefer to make it (so you can get it exactly how you think it should be / get your username is attached to the commit)?

Originally, the plugin didn’t require a key at all, like this, which is why you see it allowing any key. As the Astro VSCode extension changed, the plugin adapted. I would have loved more input from the Astro maintainers, so I’m grateful you took the time to point this out.

@louiss0
Copy link
Contributor Author

louiss0 commented Jan 29, 2023

Hi, @louiss0,

Thanks for letting me know about this. I’ll put these suggestions into a PR. Would you be willing to review that PR? Or would you prefer to make it (so you can get it exactly how you think it should be / get your username is attached to the commit)?

Originally, the plugin didn’t require a key at all, like this, which is why you see it allowing any key. As the Astro VSCode extension changed, the plugin adapted. I would have loved more input from the Astro maintainers, so I’m grateful you took the time to point this out.

I would like to review the PR and you make it?

@louiss0
Copy link
Contributor Author

louiss0 commented Jan 29, 2023

I think I have found the best solution for the for loop and children problem. It's the best one that I could find since I can't find the type for the native for loop that is used in typescript

@louiss0
Copy link
Contributor Author

louiss0 commented Feb 1, 2023

A pull request was created #2

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

2 participants