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

feat: index.html configurability and storybook support #18242

Merged
merged 12 commits into from
Oct 8, 2021

Conversation

ZachJW34
Copy link
Contributor

@ZachJW34 ZachJW34 commented Sep 25, 2021

This PR enables three things:

  1. Allow user provided index.html for both webpack-dev-server and vite-dev-server
  2. Generate custom Cypress config and index.html based on if user has Storybook configured
  3. Generate a runnable spec from a Storybook story

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 own index.html (with the support of live-reloading) made sense and gave us the option to inject the users preview-head.html and preview-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 using react-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 the Generate Config are more for show (and only for a CRA app for now).
Screen Shot 2021-10-06 at 4 28 59 PM

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 steps

React

In a separate terminal (cypress root):

  • cd npm/react/examples/react-scripts-typescript
  • npx sb init && yarn add -D @storybook/testing-react
  • Edit cypress/plugins/index.js to add indexHtml
    devServer(on, config, { indexHtml: 'cypress/component/support/index.html' })
    
  • Add a .storybook/preview-head.html and put some styles in there
    <style>
      body {
        background-color: red;
      }
    </style>
    

In a separate terminal (cypress root):

  • yarn dev --project npm/react/examples/react-scripts-typescript
  • Choose 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 the cypress.json hit back and regen config -> Choose a Browser Chrome and Launch
  • In browser, navigate to http://localhost:8080/__vite__/#/newspec
  • Choose a spec from the list of detected specs (Button, Header, or Page)
  • Click on newly generated spec (if it doesn't load try refreshing the browser, sometimes the runner doesn't pick it up)
  • Play around with the new spec. You can modify npm/react/examples/react-scripts-typescript/cypress/component/support/index.html and see the live-reloads. You'll notice that the preview-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
  • Edit cypress/plugins/index.js to add indexHtml option (since cypress.config.js isn't supported yet) and runtime compiler
    const modifiedWebpackConfig = {
        ...webpackConfig,
        plugins: (webpackConfig.plugins || []).filter((x) => {
          return x.constructor.name !== 'HtmlPwaPlugin'
        }),
        resolve: {
          ...webpackConfig.resolve,
          alias: {
            ...(webpackConfig.resolve || {}).alias,
            vue$: require.resolve('vue/dist/vue.esm-bundler.js'),
          }
        }
      }
    return startDevServer({
      options,
      webpackConfig: modifiedWebpackConfig,
      indexHtml: 'cypress/component/support/index.html'
    })
    
  • Add a .storybook/preview-head.html and put some styles in there
    <style>
      body {
        background-color: red;
      }
    </style>
    
  • Change cypress.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 the Install 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)

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 25, 2021

Thanks for taking the time to open a PR!

if (this.ctx.activeProject?.isFirstTimeCT) {
const indexHtmlPath = path.resolve(this.ctx.activeProject.projectRoot, 'cypress/component/support/index.html')

fs.outputFileSync(indexHtmlPath, template)
Copy link
Contributor

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({
Copy link
Contributor

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.

Copy link
Contributor Author

@ZachJW34 ZachJW34 Oct 6, 2021

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

@ZachJW34 ZachJW34 force-pushed the storybook-project-support branch 2 times, most recently from 44773ee to 9932935 Compare October 6, 2021 21:01
@ZachJW34 ZachJW34 marked this pull request as ready for review October 6, 2021 22:49
Copy link
Member

@tgriesser tgriesser left a 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

@ZachJW34 ZachJW34 force-pushed the storybook-project-support branch 3 times, most recently from ab75ef2 to 8f70b2a Compare October 7, 2021 20:22
@ZachJW34 ZachJW34 changed the title Storybook project support feat: index.html configurability and storybook support Oct 8, 2021
Copy link
Contributor

@lmiller1990 lmiller1990 left a 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}`
Copy link
Contributor

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.

Copy link
Contributor Author

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) {
Copy link
Contributor

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> {
Copy link
Contributor

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

Copy link
Contributor Author

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.

@ZachJW34 ZachJW34 merged commit 745b3ac into unified-desktop-gui Oct 8, 2021
@ZachJW34 ZachJW34 deleted the storybook-project-support branch October 8, 2021 18:16
tgriesser added a commit that referenced this pull request Oct 10, 2021
* 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)
  ...
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

Successfully merging this pull request may close these issues.

None yet

4 participants