-
Notifications
You must be signed in to change notification settings - Fork 3.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
feat: index.html configurability and storybook support #18242
Conversation
Thanks for taking the time to open a PR!
|
ec7f813
to
6273075
Compare
if (this.ctx.activeProject?.isFirstTimeCT) { | ||
const indexHtmlPath = path.resolve(this.ctx.activeProject.projectRoot, 'cypress/component/support/index.html') | ||
|
||
fs.outputFileSync(indexHtmlPath, template) |
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.
Currently we are doing some dependency injection with packages and they are being injected in the context. To use fs-extra it would be this.ctx.fs.outputFileSync()
@@ -0,0 +1,31 @@ | |||
import { objectType } from 'nexus' | |||
|
|||
export const WizardStorybook = objectType({ |
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.
I don't know if we should prefix this with Wizard. When we query for this it will already be in the Wizard object so we'll know where it's at.
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.
I was following the pattern that any nested types of Wizard were prefixed with Wizard
(for example, WizardBundler
, WizardFrontendFramework
). I can change it to Storybook
easily enough.
Edit: I renamed it to Storybook
44773ee
to
9932935
Compare
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.
See comment on data/util/storybook
ab75ef2
to
8f70b2a
Compare
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.
Nice job figuring out all the GQL stuff, left a few minor comments. I did not get to test it locally yet.
|
||
// Runner doesn't pick up new file without timeout, I'm guessing a race condition between file watcher and runner starting | ||
setTimeout(() => { | ||
window.location.href = `${window.location.origin}/__/#/tests/component/${generatedSpec}` |
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.
Little weird, good to keep this in ind and see if we can find a more reliable fix than just setTimeout
and hope for the best, but probably okay for now.
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.
Yeah I knew this was bound to change when the official UI lands and the runner is split apart so I was ok with the hack for demo purposes
@@ -173,4 +173,18 @@ export class ProjectActions { | |||
async clearLatestProjectCache () { | |||
await this.api.clearLatestProjectsCache() | |||
} | |||
|
|||
async createComponentIndexHtml (template: string) { |
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.
Does not need to be async
private async generate ( | ||
storyPath: string, | ||
projectRoot: string, | ||
): Promise<Cypress.Cypress['spec'] | null> { |
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.
I think we should slowly move away from the global type-defs, you could use one of the types in @packages/types/src/specs.ts
for a Spec.
Also you can just do Cypress.Spec
, but let's try to use the @packages/types
where possible. Maybe you can grab this one interface and add it in here https://github.com/cypress-io/cypress/pull/18406/files#diff-62aad6616ccf4d3d61c8140542419a8575e08a9377a32c7c2d89624ba1a0ddf0R17
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.
I used FoundSpec
from @packages/types
instead and added the extra data onto the file. Might be useful later on.
8f70b2a
to
efd58a7
Compare
efd58a7
to
51c4a7c
Compare
* unified-desktop-gui: (40 commits) feat: index.html configurability and storybook support (#18242) fix: remove .json check from require_async, prevent child_process spawn (#18416) percy snapshot the tooltip visually, prevent it from being hidden fix: failing tests from #18372 (#18414) fix: `everyNthFrame` should only be applied for Chrome 89+ (#18392) feat(app): render spec list, command log, iframe (#18372) fix: drag and drop to be correct directory (#18400) refactor: Add GitDataSource, FileDataSource, toast for errors (#18385) docs: General updates to contributing guide (#18283) Add shorter --ct alias for --component Add --e2e and --component CLI options chore: Update Chrome (beta) to 95.0.4638.40 (#18389) chore: use circleci timings split for e2e tests (#18367) fix: fixed title (#18370) chore(deps): update dependency electron to v14 🌟 (#18384) chore(server): share client route (#18215) fix: Prevent Cypress from crashing when argument parsing "spec: {}" (#18312) chore: update husky dev dependency to v7 (#18345) feat: add defineConfig function to help type config (#18302) chore: Update Chrome (stable) to 94.0.4606.71 (#18324) ...
This PR enables three things:
index.html
for bothwebpack-dev-server
andvite-dev-server
The addition of the
indexHtml
option for both dev-servers developed as a result of needing to allow users to add global scripts/fonts/styles to their component. Storybook allows the customization of the template in which their stories are rendered and we wanted to detect and inject those styles if they existed. Allowing the user to customize their ownindex.html
(with the support of live-reloading) made sense and gave us the option to inject the userspreview-head.html
andpreview-body.html
if detected. Win-Win.Cypress config changes can be made depending on whether Storybook is detected. For example, the

.storybook
directory needs to be flagged for transpilation when usingreact-scripts
and the vue runtime compiler can be added to the webpack config. Since their are a lot of dev-server changes along with the cypress config, the changes made to theGenerate Config
are more for show (and only for a CRA app for now).For code gen, I'm piggybacking off of some great Storybook packages. csf-tools allows you to parse a story file, and @storybook/testing allows you to import and render stories.
How to Test
For the complete flow, the
cypress.config.js
and dev-server changes need to be finalized but you can test this with a few manual stepsReact
In a separate terminal (cypress root):
cd npm/react/examples/react-scripts-typescript
npx sb init && yarn add -D @storybook/testing-react
cypress/plugins/index.js
to addindexHtml
.storybook/preview-head.html
and put some styles in thereIn a separate terminal (cypress root):
yarn dev --project npm/react/examples/react-scripts-typescript
Component Testing
->Frontent Framework
Choose Create React App ->Install Dev Dependencies
->Cypress.config
(notice indexHtml and storybook specific option since storybook was detected) ->Initialize Config
This will probably fail, just delete thecypress.json
hit back and regen config ->Choose a Browser
Chrome and Launchhttp://localhost:8080/__vite__/#/newspec
npm/react/examples/react-scripts-typescript/cypress/component/support/index.html
and see the live-reloads. You'll notice that thepreview-head.html
was injected into this generated template.Video
https://user-images.githubusercontent.com/25158820/136292610-6d4068a3-9499-4089-9b1e-0980efd90abc.mov
Vue
This same flow can be tested with a vue-cli (
npm/vue/examples/vue-cli
) app with a few caveats:In a separate terminal:
cd npm/vue/examples/vue-cli
npx sb init && yarn add -D @storybook/testing-vue3
cypress/plugins/index.js
to addindexHtml
option (since cypress.config.js isn't supported yet) and runtime compiler.storybook/preview-head.html
and put some styles in therecypress.json
spec pattern to"testFiles": "**/*cy-spec.js",
The launchpad flow will be the same (you can choose Vue-CLI rather than Create-React-App)
A lot of these manual steps can be alleviated when the cypress.config.js changes are in and we can generate custom configs that will be picked up.
Improvements
@storybook/testing-react
and@storybook/testing-vue3
have to be manually installed. This can be added to theInstall Deps
page or we bring the packages in house.Storybook's
preview.js
isn't currently being injected into the component support file. This isn't hard to do but there isn't any groundwork for generating a component-specific support file so I left it out of this PR.Better UI (will come shortly after once mocks are finished)