-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
WIP Add support for idiomatic svelte3 stories #7023
Conversation
This pull request is automatically deployed with Now. Latest deployment for this branch: https://monorepo-git-fork-rixo-svelte3.storybook.now.sh |
@rixo Thanks for putting this together!!! Interesting solution to a thorny problem. 😄 @Hypnosphi @ndelangen @tmeasday @igor-dv I'm wondering whether this might be a good candidate for addon-style framework support that @Hypnosphi is pioneering right now in #6826 |
From my perspective, I'd much prefer to write the stories using svelte semantics. However, I do wonder if this is something that should be brought into the core library versus an addon. Bringing a framework specific implementation to how to write stories into the library seems like a potential complexity increase. I'll still go through and see what I find regarding the provided approach. |
Reading through it again, I'm wondering whether this would be a better fit for |
Wow, this is really exciting @rixo ! Are users supposed to add an additional file for this, next to their component, or should/can this be part of the component file itself? @shilman I see how this related to docs-mode and the mdx work being done, but this is different if the stories are defined within the component-code. If not, then it probably makes sense to use mdx instead perhaps? Because that will be very similar. Assuming mdx and the docs-mode work is not really compatible with this, here's some feedback on this PR: I wonder if there's truly no way to merge app/svelte & app/svelte3. Secondary I wonder if we could get rid of that storiesOf call all-together, because it looks odd and unnecessary. This would look quite a bit cleaner IMHO: <script>
import { Story } from '@storybook/svelte3';
import Button from './Button.svelte';
</script>
<Story name="components|Button/default">
<Button />
</Story>
<Story name="components|Button/rounded">
<Button rounded />
</Story> But that ☝️ is pretty much the syntax of what's in the works for mdx support! @shilman |
Thanks for all the feedback. This is very similar to It would even be easier in some respects. There's the question of integrating with the existing
This is to be used in separate files, as usual. I didn't really consider the alternative. It would be really hard to implement; my current approach wouldn't work for this. It would also be hard to reflect about where and what is what. And since Svelte 3 enforces a strict 1 component per file, no escape hatch, that would compete for some already precious space in the main component file.
I don't know much about mdx, but I'm not sure it would cut it. The main purpose of this proposal is to be able to use all of svelte's features directly in the story. Because svelte components, once compiled, do not expose the entirety of the API they have when used in "svelte context", in the templates. Most notably, slots are not available (that would be like not being able to use React's So there is a real pain point in writing "normal" stories with Svelte, that I'm trying to address here. My expected use case was not the same as sb-docs, even if the syntax ends up extremely similar (and even if I find sb-docs very exciting in its own respect -- having previously experimented with stories in markdown myself, with ES6 template strings... And eventually finding it hard to cleanly cut the concerns of the storybook, between style guide, visual / snapshot testing, and documentation.). Anyway... From what I get, MDX will run JSX, not Svelte, so it won't address the issue with Svelte stories authoring. Are there any plan on supporting inclusion of snippets in other syntaxes? Svelte's compiler would probably lend itself very well to turning small strings of code into renderable things to be displayed as doc blocks, if that's the only job that is asked from it. But even so, that would be great for the style guide / docs parts, but that wouldn't solve the original issue with authoring stories. (Correct me if I'm wrong, which is very possible.)
Ah, that would be great! We really only need it to bind the Meanwhile, I've actually already experimented with some fully declarative syntax that does without the But in the current state of affair, it suffers from technical limitations on the svelte part. The compiler spits out a very annoying warning when using
There are some ways actually. But is it worth the added complexity to both projects? Is this desirable to be able to use both at once? For example, we can renounce on abusing import { Story, StoriesOf } from '@storybook/svelte'; // <= to be used in .svelte
import { storiesOf } from '@storybook/svelte'; // <= to be used in .js The current role I've ascribed to // maybe use `set` to denote that there's an underlying persistent side effect
Story.set(kind, module).addDecorator(...).addParameters(...);
Story.set(kind); // partial contextualisation, to be completed with props in template
Story.set(module); |
Can a <script>
import { Story } from '@storybook/svelte';
export default {
title: 'components|Button',
module,
decorators: [] // array of functions
}
</script>
<Story name="default">
<Button />
</Story>
<Story name="rounded">
<Button rounded />
</Story> If we don't have a storiesOf call needed, it can easily go in the same package. I have a strong preference for keeping it in the same package; from an organisational standpoint for this repo and also for users, who might be using both syntaxes for whatever reason. alternative: import { Story, addHmr } from '@storybook/svelte'; // <= to be used in .svelte
import { storiesOf } from '@storybook/svelte'; // <= to be used in .js |
@rixo What's the difference between JSX and Svelte syntax? Here's what I have in mind for SB Docs: import { Story } from '@storybook/addon-docs';
# Some markdown
<Story name="my story">
...story contents here
</Story> I think it's possible to parse Here's what an angular story looks like, for example: <Story name="to storybook" height="300px">
{{
template: `<storybook-welcome-component (showApp)="showApp()"></storybook-welcome-component>`,
props: {
showApp: linkTo('Button'),
},
moduleMetadata: {
declarations: [Welcome],
},
}}
</Story> Where <Story name="to storybook" height="300px" props="..." moduleMetadata="...">
<storybook-welcome-component (showApp)="showApp()"></storybook-welcome-component>
</Story> Just spitballing here... We might need to fence the contents somehow: <Story ...>{/*
...
*/}</Story> |
@ndelangen Oh that's interesting. They cannot export So we could have something like this: <script>
import { Story } from '@storybook/svelte';
// I think keeping the tie to storiesOf, even if just the name, may be better for
// transfer of experimented SB users knowledge, since the function remains the same
export const storiesOf = {
title: 'components|Button', // <= isn't 'kind' the consecrated term for that?
module,
decorators: [] // array of functions
}
</script>
<Story name="default">
<Button />
</Story>
<Story name="rounded">
<Button rounded />
</Story> There's also a way to have real named exports in svelte, by using a But, you've got me thinking... I did manage to scrap the hard requirement to pass the module with some webpack black magic. I'm sorry for going a little off topic, but I've so often been bitten by forgetting to pass the module that I can't help but elaborate. I was able to do that because I used a custom loader for svelte stories (just letting the user's code requiring the module was not enough for svelte stories, I needed to also instantiate the component for it to reveal its content... I intended to get your opinion on this point BTW, since it departs from Storybook's usual way). Anyway, since it's my code on the Storybook side that does the require, and since all story registering is synchronous, I am able to reliably pair the module id (filename) and the kinds / stories it has added. Webpack's HMR API doesn't offer any way to detect the disposal of any random module except self, as far as I know, so I added a custom loader that appends a snippet to all What are you thoughts on that? Losing the ability to do random requires in my stories loader and having to return a require.context from it instead, in exchange for not having to pass the module to every call of @shilman Well the syntax itself is very similar. There are some minor differences, like Svelte has support for some directives like But the most important distinction is not really the syntax itself, it's how they are processed. JSX transpiles to JS that is directly inlined in the surrounding code. This is not the case at all with svelte. Svelte code cannot be transpiled, it requires compilation. There's a whole lot of code generated in the process, and the output can vary depending on the options you feed the compiler (whether you want a component that renders to DOM, or to HTML strings for SSR, or a WebComponent, etc.). In the end, Svelte compiler expects to take in a whole module and outputs the code for a whole module, from which only the root component (a class) is exported (well you can have some custom named exports too but that's not really relevant here). Something that could complicate things a lot is that Svelte's compiler outputs some ES6+ full of But you've got me thinking too... After some tests, it turns out that Svelte components are more transportable than I though they were until now. It turns out you can take a compiled svelte component, pass it through props to another component, and use it seamlessly in the context of this other component (I know, this sounds basic when coming from a React background). What this means is that the current integration in |
Passing the Here's where this The storystore is exposed to the global scope too:
☝️ mainly for integrations partners like chromatic, but useful in this case too I think. In fact, I'd say in the future storybook should no longer be responsible for enabling HRM on modules, loaders should. Some day... maybe V6.. |
@rixo I invited you to the Storybook GitHub organisation. I hope you accept and become part of the storybook team. We really believe in a cross framework tool, and having people enthusiastic about web components & svelte on the team would help make sure those would not get left behind, and their interests are heard. |
What I was trying to say is that I did an experiment with a webpack loader and, indeed, it allowed to not require the user from passing the module itself, and still have working HMR. That's what I called black magic, not what you're currently doing. (I would have called it that anyway. Everyhing HMR is black magic. Just ask the people using it.) But it was made possible only because I moved the To sum it up, the requirement would be either passing the module, OR special stories loader + webpack loader (and, not currently relevant but might become a future concern, all story registering strictly synchronous). Both can coexist, so that wouldn't even be a breaking change. Maybe v5 ^^ ? Should we try it with the svelte integration? Can I? Please? (Passing the module is my least favorite part of SB -- especially since SB5 removed all my other least favorite parts.) To illustrate, that would allow the lean syntax you initially suggested: <script>
import { Story, StoriesOf } from '@storybook/svelte3';
import Button from './Button.svelte';
</script>
<StoriesOf kind="components|Button">
<Story name="default">
<Button />
</Story>
<Story name="rounded">
<Button rounded />
</Story>
</StoriesOf>
<Story kind="components|SpecialButton" name="rounded">
<Button special />
</Story> Yummy.
Oh thanks, that's a honor :) I'm not at all into web components though, but if that's ok with you, I can be the svelte guy. |
This is soooo much better than what I made. Thanks for doing this @rixo, it looks amazing. |
@rixo you're the svelte guy now 🌞. I've invited you to the storybook organisation, feel free to make the modifications you'd like. |
@ndelangen Thanks again. I need to get on top of my schedule again, currently... Then I plan to merge everything in I'll try to see how far I can get with replacing JS with TS in the process. I'm not at the top of my game in TS unfortunately, so maybe I'll need to focus on stabilizing API, & making something releasable (+ docs) first. And so... It has not been said explicitely yet, so can I assume that you'll be interested in taking this PR in, with both current and new svelte3 proposed syntax merged into |
@rixo the future of svelte is svelte 3, so the future of storybook for svelte is also for svelte 3. I think it's fine to move forward with this and make @storybook/svelte behave like this. If we could preserve backwards compatibility somehow, that'd be great though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rixo @ndelangen This looks awesome. It's a huge upgrade for svelte, and the PR is really well done.
However, I've got a big request. We recently added a new story format called the Module story format (see #7110), and I believe that all new formats, including Svelte, should compile into the module format.
Module format is intended to fully replace the underlying storiesOf
API and become a standard for specifying examples of component usage. It makes examples available not only to Storybook, but also to testing tools and any other new tool that understands it.
There's already an example of this for MDX in #7145. The following lines both evaluate transparently to module format:
load(require.context('./stories', true, /\.stories\.mdx$/), module);
import stories from 'my.stories.mdx';
Doing something similar for .stories.svelte
would make Storybook for Svelte future-proof and also set a good example for any other idiomatic implementation we add in the future. It will also significantly reduce the maintenance burden on the Storybook side.
In addition, I'd suggest adopting something similar to MDX's Meta
tag, which compiles down to a default export in the module format:
<Meta
title="Addons|Docs/mdx"
decorators={[storyFn => <div style={{ backgroundColor: 'yellow' }}>{storyFn()}</div>]}
parameters={{ component: Button, notes: 'component notes' }}
/>
<Story name="hello story">
<Button onClick={action('clicked')}>hello world</Button>
</Story>
...
NOTE: FYI there's one minor incoming change to all of this to making naming consistent #7164
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL that previous review was meant to be a change request 😅
cc @tmeasday
@rixo Missed a comment or two in the thread above, but re-read just now. I think your Also, if it isn't already obvious, I'd like to make Svelte3 and MDX formats look as close to one another as possible to set clear precedent for other formats that might come in (e.g. the Vue example that somebody linked to earlier). I'm open to the possibility of changing the MDX format if it makes that possible. (That said, I'd also like to ship MDX ASAP! 😅) |
@shilman OK that's a lot to ingest! So, if I understand correctly, that means the following needs to be done for Svelte:
I think the first part should be straightforward enough, since I can follow your existing implementation. For the second part, I totally agree that MDX, Svelte and any other such integration should share the same API (the component story API), in the limits of what the target "language" permits. I find it pretty natural that MDX should be the reference implementation since it leads the way. So I'll begin with aligning with what you currently have, then I'll follow evolution along, if they come. There's the case of Now, that's just to voice my opinion because I've myself tinkered with the subject. But I don't really have a strong opinion on this point either way. So just confirm to me which way you'll go (or go first), and I'll follow suit. In any case, I can't think of a case that would require MDX to change to accommodate support for Svelte. So don't wait for me on this. Even, that's all the better for me if the interface I implement has been released already ;) Finally, the last point is the support of Svelte syntax directly into MDX files. Correct me if I'm wrong, but in practice that would more or less complete support for docs addon, wouldn't it? That's really motivating because I'm all sold on what you're working to do there. I'm convinced it will be a huge added value to SB to include official, standardized, tooling to unlock the docs & style guide power of stories. 😍 Unfortunately I'm not too sure how well that will work with Svelte. There's the problem that MDX supports JSX, not Svelte. And Svelte, even if close, is not JSX (I now see where you were going with your question, the other day). I would need to dig into MDX to see if I can find something, but I don't really hope to find anything better than your suggestion of Svelte in guarded strings. Not too exciting, I have to confess. Meanwhile, someone has already made a MDX clone for Svelte. And it's apparently gathering some steam. Do you think that could be a solution? |
@rixo Yeah, I think you got it right.
For more color on 1b, here are some compilation examples: |
Clarifying my position using your example from above: <script>
import { Story, StoriesOf } from '@storybook/svelte3';
import Button from './Button.svelte';
</script>
<StoriesOf kind="components|Button">
<Story name="default">
<Button />
</Story>
<Story name="rounded">
<Button rounded />
</Story>
</StoriesOf> This would get loaded by a webpack loader and transformed into: import Button from './Button.svelte';
export default {
title: 'components|Button'
}
export const defaultStory = () => xxx;
defaultStory.story = { name: 'default' };
export const rounded = () => yyy; Where |
Closed in favor of: #7682 |
Issue: #6653
What I did
added a new package in app/svelte3
copied existing svelte example to example/svelte3-kitchen-sink and adapted them to the new syntax
With the new package, stories are written directly as svelte components in
.svelte
files, as described in the linked issue. It supports both a shortcut syntax for files with a single kind of stories, and a complete syntax, with aStoriesOf
component, to support edge cases and more closely stick to usual storybook syntax.I don't like having a new package for svelte but the codebases of the existing svelte app and my new svelte3 are much different and probably irreconcilable. Furthermore, existing package supports svelte v2, which my version won't be able to do. So I have no idea what should be the proper course of action here, so I preferred not to touch the existing package until I get your input.
How to test
See the
svelte3-kitchen-sink
example.Addons support
Examples for addons
actions
,backgrounds
,knobs
,links
, andnotes
are included and functional.An implementation of the
centered
addon is included in the app package because I didn't manage to find how to integrate with the actualcentered
package yet.The
storysource
addon is not supported currently.Other addons have not been tested.
TODO
Tests, docs, some addons support...
Also, I think current compilation / webpack options are quite fragile. In particular, I had some interop issues with svelte compiler and the existing babel compilation. As a result, the
app/svelte3/dist
contains raw.svelte
files that are only built in the (user's) storybook app; and svelte compiler does not transpile to ES5, so I'm pretty sure it won't work in IE11 and the like. I'll probably need assistance to fix that. I have had a pretty hard time with handling the build / repo, I must confess.