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

Vite: Add builder-vite, react-vite, and vue3-vite #19007

Merged
merged 74 commits into from
Aug 25, 2022
Merged

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Aug 24, 2022

Issue:

Branched from #19002

What I did

  • Add builder-vite to monorepo
  • Add react-vite framework
  • Add vue3-vite framework
  • Remove react/vue3-specific code from builder
  • Updated vite builder

How to test

  • CI passes
  • Locally:
yarn task --task smoke-test --template react-vite/default-js --force
yarn task --task smoke-test --template vue3-vite/default-js --force

IanVS and others added 30 commits August 12, 2022 23:25
They're copied mostly straight, as-is from their old versions.  Adjustments will
be made in subsequent commits.
These were all implicit dependencies before, which caused problems for pnpm
They need to be accessible from dist, as well
It's causing the cjs file to always be chosen, which breaks in the browser.
It seems to have broken, maybe after 631795b
Not working yet
@IanVS IanVS changed the title Vite frameworks react Add vite builder and react-vite Aug 24, 2022
@IanVS IanVS changed the title Add vite builder and react-vite Vite: add builder and react-vite Aug 24, 2022
@IanVS
Copy link
Member Author

IanVS commented Aug 24, 2022

I traced down the error to the vue3 renderer. It was trying to pass an argument to .unmount(), which was removed in vuejs/core#2601, which first went out in vue 3.0.6, so updating off of 3.0.0 was breaking tsup's DTS creation.

In fact, if you look at the circleci build job: https://app.circleci.com/pipelines/github/storybookjs/storybook/28586/workflows/403fcf57-c793-489d-8ad4-556916d15c99/jobs/404987, you'll see that it's green, but if you check the bootstrap task, there are NX errors. I think that if NX fails to build a package, it should fail the CI job.

@IanVS
Copy link
Member Author

IanVS commented Aug 25, 2022

I'm running the e2e-sandbox tests locally, and it's not crashing. All I can see is a few firefox specs failing, but that doesn't seem like what's happening in CI.

Another thing I've noticed, when I first open up localhost:8001, I get an error: Error: Cannot render docs before preparing. But, refreshing the page is fine. It seems to only happen when there's no ?path= in the URL.

Edit: maybe the firefox failures are what's happening in CI after all: https://app.circleci.com/pipelines/github/storybookjs/storybook/28590/workflows/a66fc97a-f506-46fe-87bd-5cdf91ede154/jobs/405073/artifacts

What seems to be happening in the failing tests is that the navigation to the intended story is not happening, perhaps because the navigation happens too soon, before the page is fully ready, and it stays on the introduction docs page.

@IanVS IanVS changed the title Vite: add builder and react-vite Vite: add builder, react-vite, and vue3-vite Aug 25, 2022
This reverts commit c64c27f.
@shilman shilman changed the title Vite: add builder, react-vite, and vue3-vite Vite: Add builder, react-vite, and vue3-vite Aug 25, 2022
@shilman
Copy link
Member

shilman commented Aug 25, 2022

@IanVS I updated the tsconfig.json to have the same structure as the rest of the monorepo. There are a bunch of flags I removed, and I invite you to add them back as needed, probably in a separate PR 😄

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hallelujah!

@shilman shilman changed the title Vite: Add builder, react-vite, and vue3-vite Vite: Add builder-vite, react-vite, and vue3-vite Aug 25, 2022
@shilman shilman merged commit 7b2e1a7 into next Aug 25, 2022
@shilman shilman deleted the vite-frameworks-react branch August 25, 2022 14:29
@shilman shilman added the vue3 label Aug 25, 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.

4 participants