fix: Resolve existing type issues #219

xeho91 · 2024-10-23T02:10:07Z

Things to note

Removed first generic type parameter TOverrideArgs which was incomplete.
I believe is better to focus on getting types to work correctly than adding a new incomplete feature - and we haven't got close on achieving it.
Used Cmp instead of CmpOrArgs as the base generic type parameter to simplify the type system in this addon project
use legacy syntax on tests and components related to legacy API
replace # imports (which uses map from pkg.imports) inside ./src/runtime/**/* - svelte2tsx doesn't support it (yet?) - ❤️ @paoloricciuti for detailed investigation

📦 Published PR as canary version: 4.1.8--canary.219.161a0fb.0

✨ Test out this PR locally via:

npm install @storybook/[email protected]
# or 
yarn add @storybook/[email protected]

Version

Published prerelease version: v5.0.0-next.11

Changelog

💥 Breaking Change

Fix missing @storybook/docs-tools dependency #190 (@JReinhold)
Experimental support for Svelte 5 #181 (@tsar-boomba @xeho91 @JReinhold @benoitf)

🚀 Enhancement

Restore & add support for legacy syntax #186 (@xeho91 @JReinhold)

🐛 Bug Fix

fix: Resolve existing type issues #219 (@xeho91)
Upgrade version ranges - drop support for Svelte 5 prereleases #225 (@xeho91)
fix: parameters attribute from legacy <Story> being removed #224 (@xeho91)
Fix errors at enhanceRollupError in Vite #222 (@JReinhold)
refactor: Replace deprecated context="module" with module #217 (@xeho91)
fix(pre-transform): Move stories target component import declaration from instance to module tag #218 (@xeho91)
v5: Fix tags being ignored #206 (@JReinhold)
fix(parser): Resolve autodocs tag issue and extracting rawCode #201 (@xeho91)
Replace lodash usage with es-toolkit #192 (@JReinhold)
chore: use dist folder to load the files #185 (@benoitf)

⚠️ Pushed to `next`

fix typing issue in Example.stories.svelte (@xeho91)
remove obsolete test (@xeho91)

🏠 Internal

refactor: Improve AST-related types readability & fix existing issues #209 (@xeho91)

Authors: 4

Florent BENOIT (@benoitf)
Isaiah Gamble (@tsar-boomba)
Jeppe Reinhold (@JReinhold)
Mateusz Kadlubowski (@xeho91)

This is outcome of discussion at: #216

…ook/svelte` on legacy

xeho91 · 2024-10-27T09:28:42Z

replace # imports (which uses map from pkg.imports) inside ./src/runtime/**/* - svelte2tsx doesn't support it (yet?) - ❤️ @paoloricciuti for detailed investigation

@paoloricciuti Should we create an issue on sveltejs/language-tools regarding this one?

paoloricciuti · 2024-10-27T09:31:42Z

replace # imports (which uses map from pkg.imports) inside ./src/runtime/**/* - svelte2tsx doesn't support it (yet?) - ❤️ @paoloricciuti for detailed investigation

@paoloricciuti Should we create an issue on sveltejs/language-tools regarding this one?

I'll investigate a bit better but from what I've seen the transformation is actually correct...the type is lost when TS emit the DTS.

JReinhold · 2024-10-29T08:23:28Z

examples/Button.stories.svelte

@@ -19,7 +20,7 @@
    component: Button,
    tags: ['autodocs'],
    args: {
-      children: 'Click me',
+      children: createRawSnippet(() => ({ render: () => 'Click me' })),


I don't think this is a good example we want to document, and I don't think it's needed because of line 42 below?

If it's to satisfy types, then I think we should use type modifications instead like as.

Changing to:

- children: createRawSnippet(() => ({ render: () => 'Click me' })), + children: 'Click me' as Snippet,

will produce the error:

Conversion of type 'string' to type 'Snippet<[]>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first. [2352]

So, the workaround is:

- children: 'Click me' as Snippet, + children: 'Click me' as unknown as Snippet,

Which is cumbersome. Shall we proceed with this?

I provided another alternative, which can be reverted: 104d5e0

Wait but do you want a snippet or a string? Because those are not interchangeable

Well, we want a string here, because we convert it to a snippet down in the template below:

https://github.com/storybookjs/addon-svelte-csf/pull/219/files/6b646f1b2e9a44c8e52814b23ae2d71f93797b07#diff-27706a42acb167034a3d3aa2f621b4a43e00b1bd177c0e87c2028fad8e14b398R41-R43

This procedure ensures you can easily modify the arg as a string in the Control panel, while it still being passed as a snippet to the component.
But it's slightly cumbersome, and I haven't found a way yet to support this pretty common use case better yet - ideas welcome!

@xeho91 your alternative looks cleaner, but now it doesn't allow you to modify it in Controls anymore though. but let's stick with it. 👍

JReinhold · 2024-10-29T08:24:26Z

src/index.test.ts

+        children: createRawSnippet(() => ({
+          render: () => 'Click me',
+        })),


same comment, this shouldn't be necessary

src/runtime/LegacyStory.svelte

src/runtime/Story.svelte

Source: https://svelte.dev/docs/svelte/faq#How-do-I-document-my-components

JReinhold

Still have comments, but I'm approving, and letting you decide if you want to act on them or not. 🙂

shilman · 2024-11-05T01:01:39Z

🚀 PR was released in v5.0.0-next.11 🚀

chore: remove incomplete TOverrideArgs

4c8ae96

xeho91 self-assigned this Oct 23, 2024

xeho91 added the patch Increment the patch version when merged label Oct 23, 2024

xeho91 added 14 commits October 23, 2024 10:19

remove more usages of TOverrideArgs

ad899ec

first attempt to fix tests

831b119

EXPERIMENTAL: Remove generic TMeta

f43af1b

update typescript-svelte-plugin

5c4c115

cleanup and remove MapSnippetsToAcceptPrimitives

ed5e0df

This is outcome of discussion at: #216

fix some tests

5c8c71f

Remove CmpOrArgs and use Cmp instead

44547bf

use legacy syntax on LegacyStory

f05f7de

stop using never on deprecated props

0e2fb90

add autodocs test

a46a4e6

use legacy syntax on LegacyTemplate test

caa0fcc

temporarily quiet errors on regular CSF

c1d6ac4

cleanup

92ca4db

Temporarily resolve existing type issues possibly related to `@storyb…

6b646f1

…ook/svelte` on legacy

xeho91 requested a review from JReinhold October 27, 2024 09:29

xeho91 marked this pull request as ready for review October 27, 2024 09:29

JReinhold requested changes Oct 29, 2024

View reviewed changes

xeho91 added 3 commits October 31, 2024 18:05

Merge branch 'next' into fix-types

e034039

use @component to document addon components

f7007cd

Source: https://svelte.dev/docs/svelte/faq#How-do-I-document-my-components

provide alternative to document modern example

104d5e0

xeho91 requested a review from JReinhold October 31, 2024 10:47

JReinhold approved these changes Oct 31, 2024

View reviewed changes

xeho91 linked an issue Nov 1, 2024 that may be closed by this pull request

[Bug] Svelte 5: Optional children type error #214

Closed

xeho91 added 2 commits November 1, 2024 21:39

refactor: Restore legacy StoryProps

aaf555d

remove typeof in StoryProps

53947ae

xeho91 added 3 commits November 2, 2024 11:39

Attempt to restore legacy types

13ff745

fix file extension import

5db7d1a

fix test

161a0fb

xeho91 changed the title ~~fix(types): Resolve existing type issues~~ fix: Resolve existing type issues Nov 5, 2024

xeho91 merged commit 73be782 into next Nov 5, 2024
6 of 7 checks passed

shilman added the prerelease This change is available in a prerelease. label Nov 5, 2024

This was referenced Nov 5, 2024

fix(types): Allow Snippet mapper with primitives to match undefined #215

Closed

[Bug][svelte 5]: Type 'typeof Button' is not assignable to type 'Component<any, {}, string>'. #202

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Resolve existing type issues #219

fix: Resolve existing type issues #219

xeho91 commented Oct 23, 2024 •

edited by shilman

Loading

xeho91 commented Oct 27, 2024

paoloricciuti commented Oct 27, 2024

JReinhold Oct 29, 2024

xeho91 Oct 31, 2024

xeho91 Oct 31, 2024

paoloricciuti Oct 31, 2024

JReinhold Oct 31, 2024

JReinhold Oct 31, 2024

JReinhold Oct 29, 2024

JReinhold left a comment

shilman commented Nov 5, 2024

fix: Resolve existing type issues #219

fix: Resolve existing type issues #219

Conversation

xeho91 commented Oct 23, 2024 • edited by shilman Loading

Things to note

Version

💥 Breaking Change

🚀 Enhancement

🐛 Bug Fix

⚠️ Pushed to next

🏠 Internal

Authors: 4

xeho91 commented Oct 27, 2024

paoloricciuti commented Oct 27, 2024

JReinhold Oct 29, 2024

Choose a reason for hiding this comment

xeho91 Oct 31, 2024

Choose a reason for hiding this comment

xeho91 Oct 31, 2024

Choose a reason for hiding this comment

paoloricciuti Oct 31, 2024

Choose a reason for hiding this comment

JReinhold Oct 31, 2024

Choose a reason for hiding this comment

JReinhold Oct 31, 2024

Choose a reason for hiding this comment

JReinhold Oct 29, 2024

Choose a reason for hiding this comment

JReinhold left a comment

Choose a reason for hiding this comment

shilman commented Nov 5, 2024

xeho91 commented Oct 23, 2024 •

edited by shilman

Loading

⚠️ Pushed to `next`