Skip to content

ci: ship the built themes to the PR visual gate - #5477

Merged
cixzhang merged 1 commit into
mainfrom
fix/pr-visual-build-themes
Aug 25, 2026
Merged

ci: ship the built themes to the PR visual gate#5477
cixzhang merged 1 commit into
mainfrom
fix/pr-visual-build-themes

Conversation

@cixzhang

Copy link
Copy Markdown
Contributor

pr-visual has failed on every PR since it landed in #5396#5468 and #5466 both fail at the identical step with:

Theme butter is not built (packages/themes/butter/dist/source.mjs missing) — run pnpm build before the visual gate.

loadThemeOverrides reads each theme's built dist/source.mjs, because a theme's component map is the product of defineTheme rather than a literal in its source. The job downloads the Storybook artifact, which does not carry it, and never builds anything.

The job is continue-on-error, so nothing was blocked — the signal was just dead.

Why not pnpm build in the job

That is what release-gate.yml does, and it works, but it costs a full build (~1 min locally, longer on a 2-core runner) on every component PR — which destroys the thing that makes a per-PR gate defensible: a median PR is ~16 shots, about 10 seconds. build-storybook already runs pnpm build, so the artifact is free. Filtering (pnpm -F "@astryxdesign/theme-*..." build) does not work: themes take @astryxdesign/core as a peer dependency, so the filter cannot reach it and the build fails on a missing core/dist/theme/index.js.

Verified

The upload glob's common ancestor is packages/themes, so entries are <theme>/dist/… and the download restores them in place. Simulated the round trip locally: with the dists removed loadThemeOverrides throws exactly the CI error, and after restoring the artifact it returns all seven themes.

Suggested by the session that owns .github/scripts/visual-gate/**.

The gate reads each theme's dist/source.mjs — a theme's component map is
what defineTheme returns, not a literal in its source — and pr-visual only
downloaded the Storybook artifact, so every run since #5396 died on 'Theme
butter is not built'. build-storybook already builds them, so it uploads
them rather than pr-visual paying for a second full build.
@vercel

vercel Bot commented Aug 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview Aug 25, 2026 3:44am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 25, 2026
@cixzhang
cixzhang enabled auto-merge (squash) August 25, 2026 03:44
@cixzhang
cixzhang merged commit b79f0db into main Aug 25, 2026
20 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Aug 25, 2026
@github-actions
github-actions Bot deleted the fix/pr-visual-build-themes branch August 25, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant