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

Added Story/Meta typings for Svelte #19031

Closed
wants to merge 2 commits into from

Conversation

anthonator
Copy link

Issue: Svelte renderer doesn't have support for the Story and Meta types like React and Vue do. This results in Property 'args' does not exist on type type errors when using TypeScript in stories.

What I did

I added the missing typings.

How to test

  • Is this testable with Jest or Chromatic screenshots? No
  • Does this need a new example in the kitchen sink apps? No - the Svelte kitchen sink currently does not have TypeScript examples
  • Does this need an update to the documentation? I don't think so. The only Storybook + Svelte + TypeScript documentation I could find was a blog. If I'm wrong I'm happy to update documentation if you point me in the right direction.

If your answer is yes to any of these, please make sure to include it in your PR.

@kylegach
Copy link
Contributor

Hey, @anthonator. Thanks for this effort! And congratulations on your first contribution! 🥳

A couple things of note here:

  1. Active development happens on the next branch, so your PR should target that.
  2. The next branch is on 7.0 and the team has decided to not ship new features to 6.5, so only the types-7-0 file is necessary. Please look at the other renderer's types to see how the organization has changed between 6 & 7.

Thanks again for taking the time to improve Svelte types!

@ndelangen
Copy link
Member

@anthonator could you please rebase on the next branch please? We cannot proceed with this PR otherwise. Thank you 🙏 !

@ndelangen ndelangen requested review from a team and benmccann October 5, 2022 15:07
@ndelangen
Copy link
Member

@kasperpeulen perhaps this is relevant for your work on types?
If not, please disregard me ping-ing you.

@benmccann
Copy link
Contributor

@ndelangen I'm afraid I don't know enough about Storybook to be able to review this PR. I'm happy to answer Svelte-specific questions though

@ndelangen
Copy link
Member

@JReinhold perhaps you could take a peek at this one?

@JReinhold
Copy link
Contributor

I think this looks like a pretty simple migration of React types to Svelte types so this is good! But I assume that @kasperpeulen's refactoring of the type system is going to have an effect on this, so I think his say is the most important.

@JReinhold JReinhold assigned kasperpeulen and unassigned JReinhold Oct 11, 2022
@kasperpeulen
Copy link
Contributor

@ndelangen The Svelte types are already in the 7 release, and I'm improving those this week.

So I guess the question is, do we want to backport those types to 6, otherwise, this PR is already done.

@kasperpeulen
Copy link
Contributor

Starting on better CSF3 types for svelte in:
#19512

@ndelangen ndelangen closed this Oct 17, 2022
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.

6 participants