vite-ecosystem-ci icon indicating copy to clipboard operation
vite-ecosystem-ci copied to clipboard

[WIP] Allow tests to specify a 'packageDir', add storybook 7 tests

Open IanVS opened this issue 2 years ago • 9 comments

This is an attempt to get storybook back into vite-ecosystem-ci. For now, this just adds storybook 7, though 6.5 could likely be added as well.

In order to do so, I had to add support for specifying a directory containing a package.json file, other than the repo root. I did this by adding a packageDir parameter to test option objects.

This checks out storybook's repro repository, which contains the latest storybook versions and bootstrapped projects with various frameworks, all regenerated nightly to make sure the latest versions are used.

For now, I've just added a test for storybook and react, though theoretically others could be used as well I think.

I can't get this to pass locally, but I'm not sure what's going on. The node_modules/vite/node_modules in my project seems to be full of broken symlinks.

I'm opening this up as a WIP to see what happens in CI, and potentially get feedback from @dominikg whether this seems like a decent approach in the first place.

IanVS avatar Oct 26 '22 23:10 IanVS

is there an update on this? i understand storybooks infra is built around testing releases on the registry, but would love to see this work out so we can get feedback from it before releasing a new vite version

dominikg avatar Dec 26 '22 15:12 dominikg

I rebased and pushed up the name change from packageDir to testSubdirectory, and removed an extra yarn install. However, I'm not able to run this locally for some reason. I ran nvm install 18 to update my node version, but now npm and pnpm won't work. No idea why. So I guess maybe someone else can test this out?

IanVS avatar Dec 29 '22 03:12 IanVS

if you installed pnpm via npm install -g pnpm you need to rerun that, nvm has separate global jars for each node version.

ran it locally and get an error during build-storybook

/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook $> nr build-storybook

> [email protected] build-storybook /home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook
> storybook build

@storybook/cli v7.0.0-beta.16

info => Cleaning outputDir: /storybook-static
(node:7362) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
info => Loading presets
info => Building manager..
info => Manager built (97 ms)

attention => Storybook now collects completely anonymous telemetry regarding usage.
This information is used to shape Storybook's roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://storybook.js.org/telemetry

vite v4.0.3 building for production...
transforming...
✓ 43 modules transformed.
[vite]: Rollup failed to resolve import "prop-types" from "/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/src/stories/Button.jsx".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
ERR! Error: [vite]: Rollup failed to resolve import "prop-types" from "/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/src/stories/Button.jsx".
ERR! This is most likely unintended because it can break your application at runtime.
ERR! If you do want to externalize this module explicitly add it to
ERR! `build.rollupOptions.external`
ERR!     at onRollupWarning (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/file+..+..+..+..+..+vite+packages+vite/node_modules/vite/dist/node/chunks/dep-38aa69fc.js:44627:19)
ERR!     at onwarn (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/file+..+..+..+..+..+vite+packages+vite/node_modules/vite/dist/node/chunks/dep-38aa69fc.js:44398:13)
ERR!     at Object.onwarn (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/rollup.js:24290:13)
ERR!     at ModuleLoader.handleInvalidResolvedId (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/rollup.js:22927:26)
ERR!     at file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/rollup.js:22888:26
ERR!     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
ERR!  Error: [vite]: Rollup failed to resolve import "prop-types" from "/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/src/stories/Button.jsx".
ERR! This is most likely unintended because it can break your application at runtime.
ERR! If you do want to externalize this module explicitly add it to
ERR! `build.rollupOptions.external`

personal sidenote: I did not consent to telemetry and believe it should not be enabled by default.

dominikg avatar Dec 29 '22 14:12 dominikg

also saw that the test is using react, so you might want to / have to override vite-plugin-react in addition to vite, a recent feature in vite-ecosystem-ci allows you to add it via config: https://github.com/vitejs/vite-ecosystem-ci/blob/main/tests/vite-plugin-ssr.ts#L9

This only works if the project under test is in the same monorepo structure, if you build a completely separate project, that would still use vite and vite-plugin-react from the npm registry instead

dominikg avatar Dec 29 '22 14:12 dominikg

Thanks for testing it out. npm itself is not working in node 18. If/when I get that working I'll come back to this.

Edit: it's working today, I think the npm registry was just struggling last night.

IanVS avatar Dec 29 '22 15:12 IanVS

This only works if the project under test is in the same monorepo structure

If which project is in the same structure as what else?

IanVS avatar Dec 29 '22 15:12 IanVS

This only works if the project under test is in the same monorepo structure

If which project is in the same structure as what else?

overrides are set in dir, so storybook7/package.json in this case

dominikg avatar Dec 29 '22 15:12 dominikg

Ah ok thanks, there is no package.json there in my case, so I may need to adjust that to work in testSubdirectory (if I didn't already, I can't remember what I did here).

IanVS avatar Dec 29 '22 15:12 IanVS

This already exposed a bug in storybook, which will be fixed by https://github.com/storybookjs/storybook/pull/20449 when it is released.

IanVS avatar Dec 31 '22 14:12 IanVS