diff --git a/.github/scripts/visual-gate/lib/sources.mjs b/.github/scripts/visual-gate/lib/sources.mjs index 3695985fec1fd..1945d71926472 100644 --- a/.github/scripts/visual-gate/lib/sources.mjs +++ b/.github/scripts/visual-gate/lib/sources.mjs @@ -17,10 +17,49 @@ * two lists. */ +import {execFileSync} from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import {pathToFileURL} from 'node:url'; +/** The compiled entry a built theme's own `@astryxdesign/core/theme` import resolves to. */ +const CORE_THEME_ENTRY = 'packages/core/dist/theme/index.js'; + +const BUILD_TIMEOUT_MS = 300_000; + +/** + * @param {string} repoRoot + * @param {string} pkg + */ +function pnpmBuild(repoRoot, pkg) { + execFileSync('pnpm', ['-F', pkg, 'build'], { + cwd: repoRoot, + stdio: 'inherit', + timeout: BUILD_TIMEOUT_MS, + }); +} + +/** + * @param {string} file + * @param {string} [query] - cache-buster, so a post-build retry is a fresh import + * @returns {Promise<{ok: true, module: Record} | {ok: false, error: Error}>} + */ +async function importBuilt(file, query = '') { + try { + return {ok: true, module: await import(pathToFileURL(file).href + query)}; + } catch (error) { + return {ok: false, error: /** @type {Error} */ (error)}; + } +} + +/** + * @param {string} dir + * @returns {string} + */ +function packageName(dir) { + return JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')).name; +} + /** * @param {string} repoRoot * @returns {Promise} @@ -48,25 +87,59 @@ export async function loadThemingTargets(repoRoot) { * probe tier answers in 128 with a set cover. It is a coverage instrument, not * a theme someone ships. * + * Builds what it cannot read. On CI these dists arrive as an artifact from the + * job that already built them, and three changes in one day to how that + * artifact is named and filled each reddened the gate on every open component + * PR with a missing-file error about something no PR had touched. The gate owns its own + * prerequisites now, so how they get onto the runner is an optimisation rather + * than a correctness dependency. + * * @param {string} repoRoot * @param {string} [probeTheme] - name of the coverage fixture to leave out + * @param {(repoRoot: string, pkg: string) => void} [build] - seam for tests * @returns {Promise>>} */ -export async function loadThemeOverrides(repoRoot, probeTheme = 'probe') { +export async function loadThemeOverrides(repoRoot, probeTheme = 'probe', build = pnpmBuild) { const themesDir = path.join(repoRoot, 'packages/themes'); /** @type {Record>} */ const overrides = {}; + /** @type {string[]} */ + const rebuilt = []; + + /** + * @param {string} pkg + * @param {string} why + */ + const buildOnce = (pkg, why) => { + if (rebuilt.includes(pkg)) return; + rebuilt.push(pkg); + console.log(`visual gate: ${why}; building ${pkg} here rather than failing.`); + build(repoRoot, pkg); + }; for (const entry of fs.readdirSync(themesDir, {withFileTypes: true}).sort()) { if (!entry.isDirectory()) continue; - const built = path.join(themesDir, entry.name, 'dist/source.mjs'); - if (!fs.existsSync(built)) { - throw new Error( - `Theme ${entry.name} is not built (${built} missing) — run pnpm build before the visual gate.`, - ); + const dir = path.join(themesDir, entry.name); + const built = path.join(dir, 'dist/source.mjs'); + + let loaded = await importBuilt(built); + if (!loaded.ok) { + if (!fs.existsSync(path.join(repoRoot, CORE_THEME_ENTRY))) { + buildOnce( + '@astryxdesign/core', + `${CORE_THEME_ENTRY} is missing and every built theme imports it`, + ); + } + if (!fs.existsSync(built)) { + buildOnce(packageName(dir), `${path.relative(repoRoot, built)} is missing`); + } + loaded = await importBuilt(built, '?rebuilt'); + if (!loaded.ok) { + throw unreadableTheme(repoRoot, entry.name, built, loaded.error, rebuilt); + } } - const module = await import(pathToFileURL(built).href); - const theme = Object.values(module).find(value => value?.name && value?.components); + + const theme = Object.values(loaded.module).find(value => value?.name && value?.components); if (!theme || theme.name === probeTheme) continue; overrides[theme.name] = Object.fromEntries( Object.entries(theme.components).map(([key, styles]) => [key, Object.keys(styles ?? {})]), @@ -76,6 +149,29 @@ export async function loadThemeOverrides(repoRoot, probeTheme = 'probe') { return overrides; } +/** + * @param {string} repoRoot + * @param {string} name + * @param {string} built + * @param {Error} error + * @param {string[]} rebuilt + * @returns {Error} + */ +function unreadableTheme(repoRoot, name, built, error, rebuilt) { + const rel = path.relative(repoRoot, built); + return new Error( + [ + `The visual gate could not load theme ${name} from ${rel}.`, + ` ${error.message}`, + rebuilt.length + ? ` The gate rebuilt ${rebuilt.join(' and ')} here and the import still fails, so this is a broken build rather than a missing one.` + : ` Both ${rel} and ${CORE_THEME_ENTRY} are present, so this is not a missing build.`, + `The gate reads each theme's BUILT source because a theme's component map is what defineTheme returns, not a literal in its source.`, + ].join('\n'), + {cause: error}, + ); +} + /** * @param {string} repoRoot * @returns {{excludeStories: Record, viewport: {width: number, height: number}, settleMs: number, threshold: number, maxDiffPixels: number, defaultTheme: string, probeTheme: string, tiers: string[]}} diff --git a/.github/scripts/visual-gate/lib/sources.test.mjs b/.github/scripts/visual-gate/lib/sources.test.mjs new file mode 100644 index 0000000000000..0e8181dcdf2ad --- /dev/null +++ b/.github/scripts/visual-gate/lib/sources.test.mjs @@ -0,0 +1,94 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +import {afterEach, describe, expect, it} from 'vitest'; + +import {loadThemeOverrides} from './sources.mjs'; + +/** @type {string[]} */ +const made = []; + +/** + * A repo root with `packages/core/dist` present and one theme package, whose + * built source is written only when `built` is true. + * + * @param {{built: boolean}} options + */ +function fixture({built}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-gate-sources-')); + made.push(root); + fs.mkdirSync(path.join(root, 'packages/core/dist/theme'), {recursive: true}); + fs.writeFileSync(path.join(root, 'packages/core/dist/theme/index.js'), ''); + const dir = path.join(root, 'packages/themes/butter'); + fs.mkdirSync(path.join(dir, 'dist'), {recursive: true}); + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({name: '@astryxdesign/theme-butter'}), + ); + if (built) writeBuilt(dir); + return {root, dir}; +} + +/** @param {string} dir */ +function writeBuilt(dir) { + fs.writeFileSync( + path.join(dir, 'dist/source.mjs'), + "export const theme = {name: 'butter', components: {badge: {base: {}, 'variant:info': {}}}};\n", + ); +} + +afterEach(() => { + for (const root of made.splice(0)) fs.rmSync(root, {recursive: true, force: true}); +}); + +describe('loadThemeOverrides', () => { + it('reads a built theme without building anything', async () => { + const {root} = fixture({built: true}); + /** @type {string[]} */ + const builds = []; + + const overrides = await loadThemeOverrides(root, 'probe', (_root, pkg) => builds.push(pkg)); + + expect(overrides).toEqual({butter: {badge: ['base', 'variant:info']}}); + expect(builds).toEqual([]); + }); + + it('builds the theme it cannot read, so a missing CI artifact is not a failure', async () => { + const {root, dir} = fixture({built: false}); + /** @type {string[]} */ + const builds = []; + + const overrides = await loadThemeOverrides(root, 'probe', (_root, pkg) => { + builds.push(pkg); + writeBuilt(dir); + }); + + expect(builds).toEqual(['@astryxdesign/theme-butter']); + expect(overrides).toEqual({butter: {badge: ['base', 'variant:info']}}); + }); + + it('builds core first when its dist is the thing missing', async () => { + const {root, dir} = fixture({built: false}); + fs.rmSync(path.join(root, 'packages/core/dist'), {recursive: true}); + /** @type {string[]} */ + const builds = []; + + await loadThemeOverrides(root, 'probe', (_root, pkg) => { + builds.push(pkg); + if (pkg === '@astryxdesign/theme-butter') writeBuilt(dir); + }); + + expect(builds).toEqual(['@astryxdesign/core', '@astryxdesign/theme-butter']); + }); + + it('says it tried when the rebuild does not produce a loadable theme', async () => { + const {root} = fixture({built: false}); + + await expect(loadThemeOverrides(root, 'probe', () => {})).rejects.toThrow( + /could not load theme butter[\s\S]*rebuilt @astryxdesign\/theme-butter here and the import still fails/, + ); + }); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef674e8c6c43e..b6a49daf6bf3c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -582,7 +582,12 @@ jobs: name: storybook-${{ needs.build-storybook.outputs.short_hash }} path: apps/storybook/dist + # Non-fatal: if this artifact is ever missing, renamed, or produced under + # a different key, the gate builds the themes itself. A crash here would + # be before the gate ever runs, which is how an artifact-wiring change + # reddened every open component PR twice in one day. - name: Download built themes and core + continue-on-error: true uses: actions/download-artifact@v8 with: name: dists-${{ needs.build-storybook.outputs.short_hash }}