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

Storybook spike #6583

Closed
5 tasks
JasonStoltz opened this issue Feb 8, 2023 · 12 comments
Closed
5 tasks

Storybook spike #6583

JasonStoltz opened this issue Feb 8, 2023 · 12 comments
Assignees

Comments

@JasonStoltz
Copy link
Member

JasonStoltz commented Feb 8, 2023

A time-boxed spike, or proof-of-concept, for an EUI storybook.

Needs

  • Install Storybook inside the EUI repo
  • Generate the needed Storybook configuration files and scripts
  • At least one example story (if fundamentally different than the examples that Storybook auto-generates)
  • Ensure that Storybook serves locally without issue
  • The work should be contained to a feature branch (or similarly gated) to not interfere with production work

Key discovery points

These may not be fully answerable when this issue is closed, but I want to put them on our radar as we start working on this stuff.

  • Feasibility of wholesale replacing src/docs with Storybook
  • Feasibility of publishing Storybook with Chromatic
@daveyholler daveyholler added documentation Issues or PRs that only affect documentation - will not need changelog entries and removed documentation Issues or PRs that only affect documentation - will not need changelog entries labels Feb 14, 2023
@cee-chen cee-chen self-assigned this Mar 1, 2023
@cee-chen
Copy link
Member

cee-chen commented Mar 1, 2023

From my spike today, this is an unfortunately incredibly high dev lift due to the tangle of different babel dependencies required by EUI as-is and required by storybook.

The first issue I ran into on attempting to run yarn storybook was a loose config/error (babel/babel#11622, storybookjs/storybook#12093). Configuring Storybook's babel plugins fixed the issue, but more undefined errors after that started throwing.

The only thing that got Storybook running for me locally was upgrading multiple babel packages to latest, rm -rf node_modules, rm -rf yarn.lock, and re-installing all yarn packages from scratch. Unfortunately, while doing so caused Storybook to run locally, it also broke EUI's docs and many webpack/babel dependencies with a frustratingly opaque error message of Module not found: Error: Can't resolve 'module' in '~/eui/node_modules/@babel/core/lib'

@daveyholler @JasonStoltz I think we need to reconsider our approach. Many of our different babel/webpack dependencies are only required due to the presence of our docs, which we are at some point deprecating. At that point it would make sense to do a package-wide audit for EUI and do wholesale upgrades, etc. for what is only needed for our source code and not our docs.

In the meanwhile, I think we need to come up with an interim solution of some kind that enables us to move forward with EUI+ without causing a bunch of frustrating tech debt work in EUI. My suggestions would be:

  • Create an interim eui-storybook repo that imports the latest compiled production version of EUI, and hosts/creates storybooks needed for EUI+ (note: this will by nature not be up-to-date with latest dev, but that's OK / a choice we can make based on our needs for EUI+)
  • Once EUI+ is shipped and running, we can at that point deprecate our docs and the many tooling/pipelines that go into generating them, and re-investigate:
    1. bringing back up our dependencies to the latest
    2. installing storybook on top of a (relatively) cleaner environment, and hoping that it works then
    3. migrating the separate storybooks back into the main eui repo

@daveyholler
Copy link
Contributor

@cee-chen Thanks for taking the time to vet this stuff out. This isn't what I was hoping to hear, but it is what I half-expected.

My gut says that we should take whatever approach gets us to the desired output more quickly – which sounds like that might be a separate repo that we can write stories in.

If we did explore the monorepo approach, would we look back in two years time and be glad we did it? I guess I just wanna know if that effort would be useful beyond this interim stage.

@cee-chen
Copy link
Member

cee-chen commented Mar 1, 2023

If we did explore the monorepo approach, would we look back in two years time and be glad we did it? I guess I just wanna know if that effort would be useful beyond this interim stage.

Depends on if we want to use monorepo architecture for anything else, e.g. illustrations, themes/styles, data grid, markdown editor, etc.

I think yes but I also think EUI+ has a higher priority, so if we want to just "get it done" ASAP and reconcile architecture after then separate repo would be the way to go.

@miukimiu
Copy link
Contributor

miukimiu commented Mar 2, 2023

I would vote for creating a eui-storybook repo.

But I would like to understand a few things. @daveyholler how do you envision the future repo(s) structure(s)?

This would give the eui team much more work to document. So I would vote to consider the monorepo approach. For that I think we can consider for now:

  • Having a eui-storybook repo with dependency management: yarn workspaces.
  • Then a basic structure "workspaces": ["eui", "eui-docsmobile"].
  • The eui workspace would be running storybook.
  • In eui-docsmobile workspace we would move what @davey built in design.elastic.co. So this would make the design.elastic.co repo redundant. This would also allow us (designers) to experiment the monorepo approach to document in doscmobile.
  • If we go to a non-monorepo approach we will always need to wait for a new version of EUI to be released. Then go to design.elastic.co and document.

@cee-chen and @daveyholler I'm probably overcomplicating... 🤣

So @cee-chen just do what feels right to you.

@JasonStoltz
Copy link
Member Author

@cee-chen A few thoughts to throw in the mix.

  • Instead of putting storybook into a new repo, could our docs (which we intend to deprecate anyway) go into the new repo?
  • Just for sake of getting something working in this spike, could we create a new branch, rip out the complicated parts, and add set up storybook?
  • Is it possible to isolate storybook and/or docs in a subdirectory within this project without executing our full monorepo plan as previously proposed? Like is there a gap approach in between our current state and turborepo?

FWIW, I am in favor of reviving the monorepo effort.

@cee-chen
Copy link
Member

cee-chen commented Mar 2, 2023

If we go to a non-monorepo approach we will always need to wait for a new version of EUI to be released. Then go to design.elastic.co and document.

To be clear, I believe we would need to wait for that in a monorepo as well. Separate repos within mono repos cannot (or should not, rather) rely on dev/main within another repo, it should be importing the actual published package. I'm not totally sure there is a sane or non-janky way around that without ending up in the same dependency pollution issue we're currently in.

Instead of putting storybook into a new repo, could our docs (which we intend to deprecate anyway) go into the new repo?

Not without having a bunch of additional work re-pointing/forwarding our https://elastic.github.io/eui/ production docs and rewriting our Jenkins eui.elastic.co docs as well, which I would object to as it's adding extra unnecessary tech debt effort that's wasted once we're on EUI+.

Ideally transitioning from our current EUI prod docs to EUI+ would only be done when EUI+ is "ready" to reduce the amount of downtime/impact on our dev consumers.

Just for sake of getting something working in this spike, could we create a new branch, rip out the complicated parts, and add set up storybook?

I don't fully see what this accomplishes as I believe Davey was looking to get something spun up that he could embed via iframe on EUI+.

TBH, it might make more sense to just set up & use storybook directly in the new EUI+ repo, now that I think about it.

Is it possible to isolate storybook and/or docs in a subdirectory within this project without executing our full monorepo plan as previously proposed? Like is there a gap approach in between our current state and turborepo?

Not with the same package.jsons and babel dependencies. yarn workspaces might be a gap approach between current and turborepo

@ryankeairns
Copy link
Contributor

ryankeairns commented Mar 2, 2023

TBH, it might make more sense to just set up & use storybook directly in the new EUI+ repo, now that I think about it.

This is my personal leaning, as well. Correct me if I'm wrong, but this would allow us to start writing up the stories over time and - if we decide later to move it - then at least we have the content. AND, that content can be embedded in EUI+.

Would there be a delay between releases for the component library and this new docs site? Yes. On the other hand, we could first release the Patterns/Guidelines content and could hide the components navigation until it is further built out. We could even wait to reveal that until after we sort out the EUI dependencies, monorepo considerations, and final home for Storybook.

@JasonStoltz
Copy link
Member Author

I don't fully see what this accomplishes as I believe Davey was looking to get something spun up that he could embed via iframe on EUI+.

TBH, it might make more sense to just set up & use storybook directly in the new EUI+ repo, now that I think about it.

Yep, I agree. I was just looking for a way to get something up and running for Davey. Thanks for the suggestion!

Correct me if I'm wrong, but this would allow us to start writing up the stories over time and - if we decide later to move it - then at least we have the content. AND, that content can be embedded in EUI+.

I'm fairly certain that's correct. And I think it sounds like a good first iteration and short-term solution. It sounds ideal as you'll no longer be dependent on the EUI team sorting things out in our repository.

@daveyholler do you want help from our team setting that up?

Looking out a bit longer-term, I think we on the EUI team should keep pushing on this issue and try to figure out how we're going to get storybook running inside of our repository.

@daveyholler
Copy link
Contributor

@JasonStoltz Nah, I can spin that up. I'll get it created today and share it out once it's done.

I think we're closing in on a final decision on what the architecture of the design system site should be. It will definitely require stories from EUI and I think it'd behoove us to keep pushing on the babel issues in the meantime. That way when we do launch, we're not writing stories in a separate repository.

@cee-chen
Copy link
Member

Should we either transfer this issue to another repo, or close it out (and open a new "upgrade babel to latest/storybook-supported" type issue)?

@JasonStoltz
Copy link
Member Author

@cee-chen Yes, let's close this and create a new one.

@cee-chen
Copy link
Member

#6652

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants