Skip to content

ci: build Core before inspecting downloaded themes - #5482

Merged
cixzhang merged 1 commit into
mainfrom
fix-pr-visual-core-dist
Aug 25, 2026
Merged

ci: build Core before inspecting downloaded themes#5482
cixzhang merged 1 commit into
mainfrom
fix-pr-visual-core-dist

Conversation

@rubyycheung

Copy link
Copy Markdown
Contributor

Summary

  • rebuild @astryxdesign/core in pr-visual after dependency installation
  • let the downloaded theme artifacts resolve their @astryxdesign/core/dist/* imports
  • preserve ci: ship the built themes to the PR visual gate #5477's downloaded-theme optimization without repeating the full workspace build

Why

#5477 began shipping built theme artifacts to pr-visual, but those artifacts import Core's consumer-facing dist files. The downstream job starts from a fresh checkout, installs dependencies without building packages, and therefore still fails before taking a screenshot:

ERR_MODULE_NOT_FOUND: packages/core/dist/theme/index.js
imported from packages/themes/butter/dist/source.mjs

This currently affects #5466 and #5475.

Test plan

  • removed packages/core/dist and reproduced the exact import failure from packages/themes/butter/dist/source.mjs
  • ran pnpm -F @astryxdesign/core build
  • imported the same Butter theme build successfully afterward
  • pnpm exec prettier --check .github/workflows/ci.yml
  • parsed the workflow and verified the Core build step runs after install and before the visual gate

Summary:
- rebuild Core in pr-visual after dependency installation
- let downloaded theme artifacts resolve their @astryxdesign/core dist imports
- avoid repeating the full workspace build

Test Plan:
- reproduced ERR_MODULE_NOT_FOUND with downloaded-style theme dist and absent Core dist
- pnpm -F @astryxdesign/core build
- imported packages/themes/butter/dist/source.mjs successfully after the Core build
- pnpm exec prettier --check .github/workflows/ci.yml
- parsed workflow and verified Core build runs before the visual gate
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 25, 2026
@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 6:12am

Request Review

@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
@nynexman4464

Copy link
Copy Markdown
Contributor

It looks like #5481 is the better fix, and it has now merged.

Both PRs address the same missing Core dist dependency. #5481 reuses the Core build that build-storybook already produced and ships it alongside the built themes; its armed pr-visual run completed successfully. This PR would instead rebuild Core inside every pr-visual job, adding work without improving the resulting artifact.

I think this PR can close as superseded by #5481.

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this is the half #5477 missed. I reproduced it: with core/dist removed the theme import fails exactly as CI shows, and building Core alone fixes it.

[Reviewed by Robohands]

@cixzhang
cixzhang merged commit cb20a01 into main Aug 25, 2026
20 checks passed
@github-actions
github-actions Bot deleted the fix-pr-visual-core-dist branch August 26, 2026 06:13
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.

3 participants