From 1d571e67ecdb62cef437839ef6508f5becf7ede1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:43:34 -0700 Subject: [PATCH] Speed up import checks, localization extraction, and CI typechecking (#27727) * Skip unnecessary compatibility-boundary parsing and benchmark it in CI * Use pinned preflight Bun for compatibility checks and localization extraction * Measure two concurrent Bun projects in full preflight and update its scope test --- .github/workflows/pr.yml | 7 +- .github/workflows/preflight-benchmark.yml | 45 ++++++ .../benchmark-preflight-compat-boundary.mjs | 130 ++++++++++++++++++ ...mark-preflight-localization-extraction.mjs | 96 +++++++++++++ .../scripts/check-compat-boundary-imports.mjs | 8 ++ .../check-compat-boundary-imports.test.mjs | 39 ++++++ ...alization-extraction-change-scope.test.mjs | 2 +- 7 files changed, 323 insertions(+), 4 deletions(-) create mode 100644 config/scripts/benchmark-preflight-compat-boundary.mjs create mode 100644 config/scripts/benchmark-preflight-localization-extraction.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index e2fcfe1b0f1..5f15611f0bc 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -290,7 +290,7 @@ jobs: PREFLIGHT_PRIOR_SUCCESS: ${{ job.status == 'success' }} run: | if [ "$PREFLIGHT_PHASE_SELECTED" != true ] || [ "$PREFLIGHT_PRIOR_SUCCESS" != true ]; then exit 0; fi - pnpm run typecheck:ci + pnpm run typecheck:ci --bun-concurrency=2 --bun-threads=2 - name: Check Zustand selector fan-out budget if: needs.code_paths.outputs.static_analysis == 'true' @@ -348,7 +348,7 @@ jobs: - name: Check compatibility-folder imports if: needs.code_paths.outputs.static_analysis == 'true' - run: pnpm run check:compat-boundary-imports + run: bun config/scripts/check-compat-boundary-imports.mjs - name: Check Node runtime pin if: needs.code_paths.outputs.static_analysis == 'true' @@ -356,6 +356,7 @@ jobs: # Why: extraction writes sorted evidence to an isolated temporary path, # so feature PRs need one normalized AST pass rather than a three-OS matrix. + # Use preflight's pinned Bun even when the optional local pin differs. - name: Verify localization extraction if: '!cancelled()' id: localization-extraction @@ -372,7 +373,7 @@ jobs: scope="$(node config/scripts/localization-extraction-change-scope.mjs "$RUNNER_TEMP/localization-changes")" && [ "$scope" = false ]; then echo "Localization extraction inputs are unchanged." else - pnpm run verify:localization-extraction + bun config/scripts/verify-localization-extraction.mjs fi # Why both: the ratchet proves nothing reachable from the runtime imports electron, diff --git a/.github/workflows/preflight-benchmark.yml b/.github/workflows/preflight-benchmark.yml index f141b8e6025..20432b878c6 100644 --- a/.github/workflows/preflight-benchmark.yml +++ b/.github/workflows/preflight-benchmark.yml @@ -58,3 +58,48 @@ jobs: name: preflight-static-analysis path: .build/preflight-benchmark/ retention-days: 30 + compatibility-imports: + runs-on: ubuntu-24.04-arm + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + fetch-depth: 2 + - uses: ./.github/actions/install-node-dependencies + with: + native-runtime: none + node-version: '24' + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.4.3 + - name: Compare compatibility-import scan and runtimes + run: node config/scripts/benchmark-preflight-compat-boundary.mjs + - uses: actions/upload-artifact@v7 + if: always() + with: + name: preflight-compat-boundary + path: .build/preflight-benchmark/ + retention-days: 30 + localization-extraction: + runs-on: ubuntu-24.04-arm + timeout-minutes: 12 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - uses: ./.github/actions/install-node-dependencies + with: + native-runtime: none + node-version: '24' + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.4.3 + - name: Compare localization extraction runtimes and complete catalogs + run: node config/scripts/benchmark-preflight-localization-extraction.mjs + - uses: actions/upload-artifact@v7 + if: always() + with: + name: preflight-localization-extraction + path: .build/preflight-benchmark/ + retention-days: 30 diff --git a/config/scripts/benchmark-preflight-compat-boundary.mjs b/config/scripts/benchmark-preflight-compat-boundary.mjs new file mode 100644 index 00000000000..521be683cf5 --- /dev/null +++ b/config/scripts/benchmark-preflight-compat-boundary.mjs @@ -0,0 +1,130 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' +import { measurePreflightCommand } from './preflight-benchmark-command.mjs' +import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs' +import { summarizeBenchmarkSamples } from './benchmark-sample-summary.mjs' + +const root = fileURLToPath(new URL('../..', import.meta.url)) +const output = path.join(root, '.build/preflight-benchmark') +mkdirSync(output, { recursive: true }) +const bun = process.env.ORCA_BENCHMARK_BUN ?? 'bun' +const sourcePath = 'config/scripts/check-compat-boundary-imports.mjs' +const baselineRef = process.env.ORCA_COMPAT_BASELINE_REF ?? 'HEAD^' +const baselinePath = path.join(output, 'compat-baseline.mjs') +const report = { + sourceSha: process.env.GITHUB_SHA ?? null, + baselineRef, + measurements: [], + comparisons: {}, + negativeProbe: null +} +const save = () => + writeFileSync(path.join(output, 'compat-boundary.json'), `${JSON.stringify(report, null, 2)}\n`) +const command = (program, args, label) => + measurePreflightCommand({ + program, + args, + label, + root, + output, + env: { ...process.env, ORCA_BACKGROUND_LAUNCH: '1' } + }) +async function requireSuccess(program, args, label) { + const result = await command(program, args, label) + if (result.code !== 0 || result.signal || result.timedOut) { + throw new Error(`${label}: ${JSON.stringify(result)}`) + } + return result +} +report.bunVersion = (await requireSuccess(bun, ['--version'], 'compat-bun-version')).stdout.trim() +if (report.bunVersion !== '1.4.3') { + throw new Error('Pin Bun 1.4.3') +} +report.baselineSha = ( + await requireSuccess('git', ['rev-parse', baselineRef], 'compat-baseline-sha') +).stdout.trim() +const baselineSource = ( + await requireSuccess( + 'git', + ['show', `${report.baselineSha}:${sourcePath}`], + 'compat-baseline-source' + ) +).stdout +// Both versions inspect this checkout and load the same parser and source inventory. +writeFileSync( + baselinePath, + baselineSource.replace( + /from '(\.\/[^']+)'/g, + (_, specifier) => + `from '${pathToFileURL(path.resolve(root, 'config/scripts', specifier)).href}'` + ) +) +const arms = { + baseline: [process.execPath, baselinePath], + node: [process.execPath, sourcePath], + bun: [bun, sourcePath] +} +for (const [comparison, first, second] of [ + ['compat-prefilter', 'baseline', 'node'], + ['compat-runtime', 'node', 'bun'] +]) { + let diagnostics + for (const [pair, order] of buildCounterbalancedSchedule(4, first, second).entries()) { + for (const arm of order) { + const [program, script] = arms[arm] + const result = await requireSuccess(program, [script], `${comparison}-${pair}-${arm}`) + const findings = `${result.stdout}${result.stderr}` + if (diagnostics !== undefined && diagnostics !== findings) { + throw new Error(`Diagnostics changed: ${comparison}`) + } + diagnostics = findings + report.measurements.push({ comparison, pair, arm, ...result }) + save() + console.log(JSON.stringify({ comparison, pair, arm, durationMs: result.durationMs })) + } + } + report.comparisons[comparison] = Object.fromEntries( + [first, second].map((arm) => [ + arm, + summarizeBenchmarkSamples( + report.measurements + .filter((sample) => sample.comparison === comparison && sample.arm === arm) + .map((sample) => sample.durationMs) + ) + ]) + ) + save() +} +const probeDirectory = mkdtempSync(path.join(root, 'src/shared/preflight-compat-probe-')) +try { + for (const [name, specifier] of [ + ['plain', '../workspace-layout/old-client'], + ['escaped', '../workspace-layout/old-\\u0063lient/private'] + ]) { + writeFileSync(path.join(probeDirectory, `${name}.ts`), `import '${specifier}'\n`) + } + let diagnostics + for (const [arm, [program, script]] of Object.entries(arms)) { + const result = await command(program, [script], `compat-invalid-${arm}`) + const findings = `${result.stdout}${result.stderr}` + if ( + result.code !== 1 || + result.signal || + result.timedOut || + !findings.includes('/plain.ts:') || + !findings.includes('/escaped.ts:') + ) { + throw new Error(`Compatibility check did not reject fixtures: ${JSON.stringify(result)}`) + } + if (diagnostics !== undefined && diagnostics !== findings) { + throw new Error(`Negative-probe diagnostics changed: ${arm}`) + } + diagnostics = findings + } + report.negativeProbe = 'identical findings in baseline Node, optimized Node and optimized Bun' + save() +} finally { + rmSync(probeDirectory, { recursive: true, force: true }) +} +console.log(JSON.stringify(report.comparisons)) diff --git a/config/scripts/benchmark-preflight-localization-extraction.mjs b/config/scripts/benchmark-preflight-localization-extraction.mjs new file mode 100644 index 00000000000..1dce9715b90 --- /dev/null +++ b/config/scripts/benchmark-preflight-localization-extraction.mjs @@ -0,0 +1,96 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { measurePreflightCommand } from './preflight-benchmark-command.mjs' +import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs' +import { summarizeBenchmarkSamples } from './benchmark-sample-summary.mjs' + +const root = fileURLToPath(new URL('../..', import.meta.url)) +const output = path.join(root, '.build/preflight-benchmark') +mkdirSync(output, { recursive: true }) +const bun = process.env.ORCA_BENCHMARK_BUN ?? 'bun' +const report = { sourceSha: process.env.GITHUB_SHA ?? null, measurements: [], comparison: null } +const save = () => + writeFileSync( + path.join(output, 'localization-extraction.json'), + `${JSON.stringify(report, null, 2)}\n` + ) +async function requireSuccess(program, args, label, env = process.env) { + const result = await measurePreflightCommand({ program, args, label, root, output, env }) + if (result.code !== 0 || result.signal || result.timedOut) { + throw new Error(`${label}: ${JSON.stringify(result)}`) + } + return result +} +report.bunVersion = ( + await requireSuccess(bun, ['--version'], 'extraction-bun-version') +).stdout.trim() +if (report.bunVersion !== '1.4.3') { + throw new Error('Pin Bun 1.4.3') +} +const directory = mkdtempSync(path.join(output, 'extraction-')) +const outputPattern = path.join(directory, '{{language}}.json').split(path.sep).join('/') +const args = [ + 'node_modules/i18next-cli/dist/esm/cli.js', + '--config', + 'config/i18next.config.ts', + 'extract', + '--sync-primary', + '--quiet' +] +let catalog +try { + for (const [pair, order] of buildCounterbalancedSchedule(4, 'node', 'bun').entries()) { + for (const arm of order) { + rmSync(path.join(directory, 'en.json'), { force: true }) + const result = await requireSuccess( + arm === 'bun' ? bun : process.execPath, + args, + `extraction-${pair}-${arm}`, + { ...process.env, ORCA_BACKGROUND_LAUNCH: '1', ORCA_I18N_EXTRACTION_OUTPUT: outputPattern } + ) + const extracted = JSON.stringify( + JSON.parse(readFileSync(path.join(directory, 'en.json'), 'utf8')) + ) + if (catalog !== undefined && catalog !== extracted) { + throw new Error(`Extracted catalog changed: ${arm}`) + } + catalog = extracted + report.measurements.push({ pair, arm, ...result }) + save() + console.log(JSON.stringify({ pair, arm, durationMs: result.durationMs })) + } + } + report.comparison = Object.fromEntries( + ['node', 'bun'].map((arm) => [ + arm, + summarizeBenchmarkSamples( + report.measurements + .filter((sample) => sample.arm === arm) + .map((sample) => sample.durationMs) + ) + ]) + ) + report.catalogParity = 'identical full extracted catalogs in all eight runs' + save() +} finally { + rmSync(directory, { recursive: true, force: true }) +} +let findings +for (const [arm, program] of [ + ['node', process.execPath], + ['bun', bun] +]) { + const result = await requireSuccess( + program, + ['config/scripts/verify-localization-extraction.mjs'], + `extraction-check-${arm}` + ) + if (findings !== undefined && findings !== result.stdout) { + throw new Error(`Extraction gate findings changed: ${arm}`) + } + findings = result.stdout +} +report.gateParity = findings +save() +console.log(JSON.stringify(report.comparison)) diff --git a/config/scripts/check-compat-boundary-imports.mjs b/config/scripts/check-compat-boundary-imports.mjs index 2887c12bbbf..0b57187e105 100644 --- a/config/scripts/check-compat-boundary-imports.mjs +++ b/config/scripts/check-compat-boundary-imports.mjs @@ -35,7 +35,15 @@ function importTarget(file, specifier) { export function assessCompatBoundaryImports(sources, folders = COMPAT_FOLDERS) { const violations = [] + const folderNames = folders.map(({ folder }) => path.posix.basename(folder)) for (const [file, contents] of sources) { + // Escapes can hide a folder name; relative imports can inherit it from the file path. + if ( + !contents.includes('\\') && + !folderNames.some((name) => contents.includes(name) || file.includes(name)) + ) { + continue + } for (const specifier of collectModuleSpecifiers(file, contents)) { const target = importTarget(file, specifier)?.replace(/\.[cm]?[jt]sx?$/, '') const compat = folders.find( diff --git a/config/scripts/check-compat-boundary-imports.test.mjs b/config/scripts/check-compat-boundary-imports.test.mjs index a1ecee12116..352c8e9d38a 100644 --- a/config/scripts/check-compat-boundary-imports.test.mjs +++ b/config/scripts/check-compat-boundary-imports.test.mjs @@ -47,4 +47,43 @@ describe('compatibility folder import boundary', () => { }) ).toEqual(['src/main/other.test.ts: import src/main/compat through its index, not ./compat/a']) }) + + it.each([ + './co\\u006dpat/a', + './co\\x6dpat/a', + './co\\mpat/a', + './co\\\nmpat/a', + './co\\u{6d}pat/a' + ])('rejects deep imports with escaped folder names: %s', (specifier) => { + expect(assess({ 'src/main/owner.ts': `import { a } from '${specifier}'` })).toEqual([ + 'src/main/owner.ts: import src/main/compat through its index, not ./compat/a' + ]) + }) + + it('checks const-bound loaders and dot segments', () => { + expect( + assess({ + 'src/main/other.ts': "const entry = './compat/nested/../index'; require(entry)", + 'src/renderer/src/store.ts': "export { f } from '@renderer/runtime/frozen/inner/../index'" + }) + ).toEqual([ + 'src/main/other.ts: only src/main/owner.ts may import src/main/compat', + 'src/renderer/src/store.ts: import src/renderer/src/runtime/frozen through its index, not @renderer/runtime/frozen/inner/../index' + ]) + }) + + it('checks imports inherited from a compatibility folder in the file path', () => { + expect( + assessCompatBoundaryImports( + new Map([['src/main/compat.ts/consumer.ts', "import { a } from '.'"]]), + [{ folder: 'src/main/compat', importers: [] }] + ) + ).toEqual(['src/main/compat.ts/consumer.ts: only no file may import src/main/compat']) + }) + + it('keeps parse failures fatal for candidate files', () => { + expect(() => assess({ 'src/main/owner.ts': "import { from './compat'" })).toThrow( + 'Cannot parse' + ) + }) }) diff --git a/config/scripts/localization-extraction-change-scope.test.mjs b/config/scripts/localization-extraction-change-scope.test.mjs index ab7e68a89c9..f351394e47d 100644 --- a/config/scripts/localization-extraction-change-scope.test.mjs +++ b/config/scripts/localization-extraction-change-scope.test.mjs @@ -110,7 +110,7 @@ it('preserves deleted and renamed inputs and falls back to extraction on detecti expect(step.run).toContain('git diff --name-only --no-renames -z "$DIFF_BASE" HEAD') expect(step.run).not.toContain('--diff-filter') expect(step.run).toContain('&& [ "$scope" = false ]; then') - expect(step.run).toMatch(/else\s+pnpm run verify:localization-extraction\s+fi/) + expect(step.run).toMatch(/else\s+bun config\/scripts\/verify-localization-extraction\.mjs\s+fi/) expect( affectsLocalizationExtraction(['src/old-translations.ts', 'docs/moved-translations.ts']) ).toBe(true)