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

CLI: Remove render functions and infer argTypes for svelte CLI templates #20007

Merged

Conversation

kasperpeulen
Copy link
Contributor

What I did

Updated the CLI templates for svelte

  • Removed the render functions CLI templates, as they are inferred now
  • Infer argTypes, only manual override an argType when it is different than would be inferred
  • Added TS templates

I also tried to align more with svelte conventions:

  • Forward click event with on:click
  • Don't use props for events like you would do in react

I am also adding CLI templates for TS 4.9 with satisfies for svelte, but I wanted to split this work off from the TS 4.9 PR, so that it can be reviewed seperately.

Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I didn't know the cli/ts cli/js pattern worked?

/**
* Button contents
*/
export let label = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export let label = '';
export let label;

I recommend no default, so that you test that your types picks up that it's required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch fixed!

@kasperpeulen kasperpeulen force-pushed the kasper/sb-1028-cleanup-render-function-and-manual-arg branch 3 times, most recently from 6bf375d to fe7bf94 Compare November 29, 2022 13:45
@kasperpeulen
Copy link
Contributor Author

kasperpeulen commented Nov 29, 2022

Yes, it works :) Also adding cli/ts-legacy in a new PR for TS<4.9 stuff 😅 @JReinhold

I also tried to align more with svelte conventions:
- Forward click event with on:click
- Don't use props for events like you would do in react
@kasperpeulen kasperpeulen force-pushed the kasper/sb-1028-cleanup-render-function-and-manual-arg branch from fe7bf94 to 1b08b3f Compare November 29, 2022 14:46
@kasperpeulen kasperpeulen merged commit 8ff3ce2 into next Nov 29, 2022
@kasperpeulen kasperpeulen deleted the kasper/sb-1028-cleanup-render-function-and-manual-arg branch November 29, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants