CLI: Add init support for qwik projects

[literalpie]

What I did

I added a generator for the qwik framework. It points to my library outside of the storybook mono repo storybook-framework-qwik.

How to test

• Is this testable with Jest or Chromatic screenshots?
• Does this need a new example in the kitchen sink apps? - repro template added (with some catches)
• Does this need an update to the documentation? - I will plan
  If your answer is yes to any of these, please make sure to include it in your PR.

Open questions:

• I would mostly like confirmation that my general approach isn't totally off-base.
  • I'm assuming it's preferred to keep the Qwik framework outside the storybook monorepo until it is more stable. Is this right?
  • Is it okay that I made changes to the baseGenerator to support the external framework?
  • Is it okay that my framework includes a renderer built in? Qwik is only used with Vite. Because of this, I use framework and renderer interchangeably in a few places.
• The Qwik SB framework currently fails if there are any MDX stories set. I need to find a way to remove the mdx stories from the generated main.js file fixed
• Generating the repro template fails to launch storybook when it tries "storybook not found", but running yarn storybook manually afterwards works. I don't know why this is, and need to look into it more.

[shilman]

Hey @literalpie ! Happy new year!

Thanks so much for this PR. A bunch of of us have been off for the holidays, so we haven't had a chance to discuss this yet.

At first glance, your approach seems fine to me. Main question is what are the criteria to add something to the CLI & that's what we'll discuss this week & get back to you on. If we do accept this, there's probably some cleanup we should do on the base generator before merging.

[ndelangen]

@literalpie Do you think we could have short meeting about this PR some day this or next week?

Reason being: We're worried that the storybook team will be on the hook for supporting qwik, when no-one on the team really knows how it works (yet). I'd like to gauge if you have interest in assisting with that support.

Alternatively we discussed at the triage meeting last Monday that we might modify the storybook CLI to accept a CLI-flag to pass an extension package.

Here's 1 unpolished idea:
sb init --package="storybook-qwik-init"

Meaning the storybook CLI doesn't need to know about qwik at all, what it knows instead it to call another package a certain way, that does the proper setup.
When calling this external package we could supply anything it needs from the storybook CLI package if needed.

Like I said, we're worried about over-extending ourselves and declaring support on things we know too little about, and have too few maintainers for. Changing gears to make the storybook init command extendable this way would remedy this risk for us.

Would love to hear your thoughts on this, either here in text, but I'm also very open to discussing this in a face-to-face meeting:
https://calendly.com/chromaui/

[literalpie]

@ndelangen thanks for the response. I'll pick a time in your calendar for us to meet.
I really like the idea of a CLI, but that might also be a lot of work for you, and it would be sad to miss out on the testing and other tools inside the monorepo.
There's also some work started to have the storybook integration merged into the Qwik repo, but I was trying to avoid that to keep in line with the other storybook framework support. (And I don't know if the Qwik team would be more eager to own this integration than you are 😆.
I'm on the fence on committing to help with support in the long term (I don't want to overpromise). Discussing with you will be helpful.

[shilman]

Awesome! Note that we could still test the Qwik framework in the monorepo even if it was located in its own repo/package namespace/etc. We're not quite set up for that yet, but we could make that happen if it makes sense. Obviously not as nice as just having it in the monorepo, but I think we're pretty firm on the idea that new frameworks should start somewhere else (exactly where is TBD) and then graduate to the monorepo. (We'll probably even be applying that policy to our own packages once we figure out the exact policy & build a little infrastructure to support it)

[ndelangen]

@literalpie do you think you could get this PR green. We've spoken amonst the team, and have agreed to let PRs adding templates to sb init throught! (we won't be accepting new builders/renderers/frameworks into the monorepo).

Do know that we plan to do a refactor of the storybook CLI after 7.0 has been released, and so this code will likely eventually be moved out again, when that makes sense. But right now, this can be moved forward!

Let's get it green! (please merge in next branch and fix any issues that might show.
If you need an yhelp getting this green, please don't hesitate to reach out to me (for example on discord) 👏

[literalpie]

Yes, I'll try to get this green in the next few days!

[literalpie]

@ndelangen it's green now! A couple catches to be aware of though:

• Generating the sandbox seems to always fail on the first try with an error that storybook-framework-qwik could not be found, but succeed on subsequent tries.
• The Storybook template defaults to including *.mdx files. this can cause conflicts with Qwik, which also supports MDX files, but only with Qwik components, not React components. I might try to fix this by changing the default for this generator to be *.storybook.mdx or something - I hope that doesn't cause confusion with *.stories.mdx, but I don't have any better ideas.

I can try to address the second one, either in this PR or a followup, but I don't know what to do about the first. This might also throw of the CI process if it tries to test the framework integration.

[literalpie]

adding .storybook.mdx would make the introduction.mdx file in the common template not work any more. I'm okay with keeping the mdx pattern as-is, and adding documentation to warn users to watch out for conflicts with the mdx file matching between storybook and Qwik-city.

I'm still having problems with the sandbox. I think I was wrong when I said the problem only happens once. This issue also affects the e2e tests for this sandbox. I believe the issue is that addStories in sandbox-parts.ts tries to resolve the renderer package, assuming it is in the monorepo. Maybe this should be handled in a separate issue/PR? (either after this is merged, or we can put this on hold until that is done)

[ndelangen]

@literalpie shall we plan a pairing session to work on this? Seems we're close.

I don't have solution in mind for the .mdx conflict 😞

[literalpie]

@ndelangen I think pairing on this would be helpful. I'll schedule a time with your calendar link above

[literalpie]

@ndelangen and I met, and getting e2e tests will require some changes that are out of scope of this PR. I'll add some code to exclude this sandbox from e2e tests and other parts of the sandbox setup that currently assume framework/renderer packages are inside the sb monorepo.

Once that's done, the build should pass and we should be ready to merge!

[shilman]

Great job getting this working @literalpie @ndelangen !!! ❤️

One question before I merge. What's the plan for the sandbox. I was able to run it on my dev machine, and the CLI-generated stories work great.

However, I see errors like this with the sandbox template stories:

Is the plan to leave this inDevelopment for the foreseeable future? Or do we plan to add this to our test suite soon? If so, do we have a plan for how we're going to deal with "external" frameworks in our Ecosystem CI?

[shilman]

Also this

[shilman]

Did you mean to add this to export const daily below? It's not currently getting run in CI.

[literalpie]

@shilman the global is not defined error is because the sandbox-parts file does not support loading /template/components from outside packages. @ndelangen said this was out of scope of this PR and someone else could add support for this later. This is also the reason the e2e tests are disabled.

[ndelangen]

@shilman I understand your concern, but here's my reasoning:

This is an internal maintenance issue, and not something the actual users of the CLI would ever run into.

I mentioned to @literalpie that we should simply disable the extra stories for now; we'd be adding them back and enable e2e tests then.

Let me know if you have any concerns with that. My goal was unblock the PR.

[ndelangen]

@shilman I "filtered" out the storeis coming from addons and core (because they don't work) for now.

I think we want to enable chromatic on this new sandbox.

[shilman]

LGTM! 💯