chore: implement chromatic#933
chore: implement chromatic#933jakehwll wants to merge 18 commits intojakehwll/implement-storybookfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bcc7299a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "@vscode-elements/react-elements": ^2.4.0 | ||
| "@vscode/codicons": ^0.0.45 | ||
| babel-plugin-react-compiler: ^1.0.0 | ||
| chromatic: ^16.7.0 |
There was a problem hiding this comment.
Nit: 16.8.0 is already out 😛 (also could be inline in package.json)
| run: ${{ runner.os == 'Linux' && 'xvfb-run -a' || '' }} pnpm test:integration --label "VS Code ${{ matrix.vscode-version }}" | ||
| shell: bash | ||
|
|
||
| # Reference guide: |
There was a problem hiding this comment.
I like the source but the inner comments feel overly verbose to me, we can remove or adapt this to our use-case and make them a bit slimmer!
| # 👇 Ensures Chromatic can read your full git history | ||
| fetch-depth: 0 | ||
| # 👇 Tells the checkout which commit hash to reference |
| exitOnceUploaded: true | ||
| # This will prevent CI from failing when Chromatic detects visual changes | ||
| exitZeroOnChanges: true | ||
| # Prevent excessive build runs on minor version changes |
There was a problem hiding this comment.
Misleading comment (I'd say remove most of these inline ones)
| # the check to pass. This is desired in PRs, but not in mainline. | ||
| - name: Publish to Chromatic (non-mainline) | ||
| if: github.ref != 'refs/heads/main' && github.repository_owner == 'coder' | ||
| uses: chromaui/action@latest |
There was a problem hiding this comment.
We should pin this like all other actions for security and reproducibility
| buildScriptName: "storybook:build" | ||
| # Run TurboSnap to trace file dependencies to related stories | ||
| # and tell chromatic to only take snapshots of relevant stories | ||
| onlyChanged: true |
There was a problem hiding this comment.
We should drop this on mainline because autoAcceptChanges is set to true (so we have a full baseline build)
| # This is a separate step for mainline only that auto accepts and changes | ||
| # instead of holding CI up. Since we squash/merge, this is defensive to | ||
| # avoid the same changeset from requiring review once squashed into | ||
| # main. Chromatic is supposed to be able to detect that we use squash | ||
| # commits, but it's good to be defensive in case, otherwise CI remains | ||
| # infinitely "in progress" in mainline unless we re-review each build. |
EhabY
left a comment
There was a problem hiding this comment.
Let's merge this after the storybook one lands and we have no conflicts!
| // For more info, see https://github.com/storybookjs/eslint-plugin-storybook#configuration-flat-config-format | ||
| import storybook from "eslint-plugin-storybook"; |
There was a problem hiding this comment.
This seems to be no longer maintained, maybe use the monorepo one? https://github.com/storybookjs/storybook/tree/next/code/lib/eslint-plugin
Also don't we need to extend the rules?
Follow-up to #932. Adds Chromatic to the CI pipeline so we get visual regression testing and snapshot previews on PRs.
chromaticjob toci.yamlwith separate steps for PR builds (review-gated) and mainline builds (auto-accept)onlyChanged) and zip uploads for faster, leaner buildschromaticandstorybook:ciscripts topackage.jsonchromaticdependency via the pnpm catalog