Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion benchmarks/bundle-size/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"private": true,
"type": "module",
"scripts": {
"build": "node ../../scripts/benchmarks/bundle-size/measure.mjs --skip-package-builds"
"build": "node ../../scripts/benchmarks/bundle-size/measure.mjs --skip-package-builds",
"test:unit": "node --test ../../scripts/benchmarks/bundle-size/pr-report.test.mjs"
},
"nx": {
"targets": {
Expand All @@ -23,6 +24,14 @@
"target": "build"
}
]
},
"test:unit": {
"inputs": [
"default",
"^production",
"{workspaceRoot}/scripts/benchmarks/bundle-size/pr-report.mjs",
"{workspaceRoot}/scripts/benchmarks/bundle-size/pr-report.test.mjs"
]
}
}
},
Expand Down
64 changes: 43 additions & 21 deletions scripts/benchmarks/bundle-size/pr-report.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,16 @@ async function main() {
? resolveBaselineFromCurrentJson(baselineCurrent)
: resolveBaselineFromHistory(historyEntries, args.baseSha)

const metrics = current.metrics || []
const rows = []

for (const metric of current.metrics || []) {
for (const metric of metrics) {
const baselineValue = baseline.benchesByName.get(metric.id)

if (Number.isFinite(baselineValue) && metric.gzipBytes === baselineValue) {
continue
}

Comment on lines +271 to +280

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file structure first, then inspect the relevant slice.
ast-grep outline scripts/benchmarks/bundle-size/pr-report.mjs --view expanded || true

echo '--- relevant lines ---'
sed -n '240,340p' scripts/benchmarks/bundle-size/pr-report.mjs

Repository: TanStack/router

Length of output: 4428


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect how baseline and rows are constructed, and where the "does not affect bundle size" message is emitted.
rg -n "benchesByName|get\\(|rawBytes|brotliBytes|initialGzipBytes|does not affect bundle size|metric\\.gzipBytes|baselineValue" scripts/benchmarks/bundle-size/pr-report.mjs

Repository: TanStack/router

Length of output: 1198


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,240p' scripts/benchmarks/bundle-size/pr-report.mjs

Repository: TanStack/router

Length of output: 6049


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('scripts/benchmarks/bundle-size/pr-report.mjs')
text = p.read_text()
for needle in [
    'metric.gzipBytes === baselineValue',
    'does not affect bundle size',
    'rawBytes',
    'brotliBytes',
    'initialGzipBytes',
]:
    print(f'-- {needle} --')
    for i, line in enumerate(text.splitlines(), 1):
        if needle in line:
            print(f'{i}: {line}')
PY

Repository: TanStack/router

Length of output: 572


Rows can disappear when only non-gzip metrics change scripts/benchmarks/bundle-size/pr-report.mjs:271-280

The skip check only compares metric.gzipBytes to the baseline. Since the table also shows raw, brotli, and initial, a PR can report “does not affect bundle size in any measured scenario” even when those displayed values changed. Consider including them in the change check, or remove them if gzip is the only signal.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/benchmarks/bundle-size/pr-report.mjs` around lines 271 - 280, Update
the skip condition in the metrics loop to compare every displayed metric
value—raw, gzip, brotli, and initial—against the corresponding baseline data, so
a row is skipped only when all measured values are unchanged. Preserve the
existing row-generation behavior for any metric with a change.

const historySeries = (seriesByScenario.get(metric.id) || []).slice(
// Reserve one slot for the current metric so the sparkline stays at trendPoints.
-args.trendPoints + 1,
Expand All @@ -290,6 +296,7 @@ async function main() {
raw: metric.rawBytes,
brotli: metric.brotliBytes,
initial: metric.initialGzipBytes,
hasBaseline: Number.isFinite(baselineValue),
deltaCell: formatDelta(metric.gzipBytes, baselineValue),
trendCell: sparkline(historySeries.slice(-args.trendPoints)),
})
Expand All @@ -299,30 +306,45 @@ async function main() {
lines.push(args.marker)
lines.push('## Bundle Size Benchmarks')
lines.push('')
lines.push(`- Commit: \`${formatShortSha(current.sha)}\``)
lines.push(
`- Measured at: \`${current.measuredAt || current.generatedAt || 'unknown'}\``,
)
lines.push(`- Baseline source: \`${baseline.source}\``)
if (args.dashboardUrl) {
lines.push(`- Dashboard: [bundle-size history](${args.dashboardUrl})`)
}
lines.push('')
lines.push(
'| Scenario | Current (gzip) | Delta vs baseline | Initial gzip | Raw | Brotli | Trend |',
)
lines.push('| --- | ---: | ---: | ---: | ---: | ---: | --- |')

for (const row of rows) {
if (metrics.length > 0 && rows.length === 0) {
lines.push(
`| \`${row.id}\` | ${formatBytes(row.current)} | ${row.deltaCell} | ${formatBytes(row.initial)} | ${formatBytes(row.raw)} | ${formatBytes(row.brotli)} | ${row.trendCell} |`,
'This pull request does not affect bundle size in any measured scenario.',
)
}
} else if (metrics.length === 0) {
lines.push('No bundle-size scenarios were measured.')
} else {
lines.push(`- Commit: \`${formatShortSha(current.sha)}\``)
lines.push(
`- Measured at: \`${current.measuredAt || current.generatedAt || 'unknown'}\``,
)
lines.push(`- Baseline source: \`${baseline.source}\``)
if (args.dashboardUrl) {
lines.push(`- Dashboard: [bundle-size history](${args.dashboardUrl})`)
}
lines.push('')
lines.push(
rows.some((row) => !row.hasBaseline)
? 'The following scenarios have bundle-size changes or lack baseline data for comparison:'
: 'The following scenarios have bundle-size changes compared with the baseline:',
)
lines.push('')
lines.push(
'| Scenario | Current (gzip) | Delta vs baseline | Initial gzip | Raw | Brotli | Trend |',
)
lines.push('| --- | ---: | ---: | ---: | ---: | ---: | --- |')

lines.push('')
lines.push(
'_Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better._',
)
for (const row of rows) {
lines.push(
`| \`${row.id}\` | ${formatBytes(row.current)} | ${row.deltaCell} | ${formatBytes(row.initial)} | ${formatBytes(row.raw)} | ${formatBytes(row.brotli)} | ${row.trendCell} |`,
)
}

lines.push('')
lines.push(
'_Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better._',
)
}

const markdown = lines.join('\n') + '\n'
await fsp.mkdir(path.dirname(outputPath), { recursive: true })
Expand Down
131 changes: 131 additions & 0 deletions scripts/benchmarks/bundle-size/pr-report.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import assert from 'node:assert/strict'
import { execFile } from 'node:child_process'
import { promises as fs } from 'node:fs'
import os from 'node:os'
import path from 'node:path'
import { promisify } from 'node:util'
import test from 'node:test'

const execFileAsync = promisify(execFile)
const reportScript = new URL('./pr-report.mjs', import.meta.url)

function metric(id, gzipBytes) {
return {
id,
gzipBytes,
rawBytes: gzipBytes * 3,
brotliBytes: gzipBytes - 100,
initialGzipBytes: gzipBytes - 10,
}
}

async function generateReport({ current, baseline, history, baseSha }) {
const fixtureDir = await fs.mkdtemp(
path.join(os.tmpdir(), 'bundle-size-pr-report-'),
)

try {
const currentPath = path.join(fixtureDir, 'current.json')
const outputPath = path.join(fixtureDir, 'report.md')
const args = [
reportScript.pathname,
'--current',
currentPath,
'--output',
outputPath,
]

await fs.writeFile(currentPath, JSON.stringify(current))

if (baseline) {
const baselinePath = path.join(fixtureDir, 'baseline.json')
await fs.writeFile(baselinePath, JSON.stringify(baseline))
args.push('--baseline', baselinePath)
}

if (history) {
const historyPath = path.join(fixtureDir, 'history.json')
await fs.writeFile(historyPath, JSON.stringify(history))
args.push('--history', historyPath)
}

if (baseSha) {
args.push('--base-sha', baseSha)
}

await execFileAsync(process.execPath, args)
return await fs.readFile(outputPath, 'utf8')
} finally {
await fs.rm(fixtureDir, { recursive: true, force: true })
}
}

function currentJson(metrics) {
return {
benchmarkName: 'Bundle Size (gzip)',
measuredAt: '2026-07-19T09:45:56.787Z',
sha: '126aa13f296b1234',
metrics,
}
}

test('renders a concise message when no measured scenario changed', async () => {
const current = currentJson([metric('react-router.minimal', 1_000)])
const report = await generateReport({ current, baseline: current })

assert.equal(
report,
'<!-- bundle-size-benchmark -->\n## Bundle Size Benchmarks\n\nThis pull request does not affect bundle size in any measured scenario.\n',
)
})

test('renders only scenarios that changed against the historical baseline', async () => {
const current = currentJson([
metric('react-router.minimal', 1_000),
metric('react-router.full', 1_200),
])
const history = {
entries: {
[current.benchmarkName]: [
{
commit: { id: 'baseline-sha' },
benches: [
{ name: 'react-router.minimal', value: 1_000 },
{ name: 'react-router.full', value: 1_100 },
],
},
],
},
}
const report = await generateReport({
current,
history,
baseSha: 'baseline-sha',
})

assert.match(
report,
/The following scenarios have bundle-size changes compared with the baseline:/,
)
assert.doesNotMatch(report, /`react-router\.minimal`/)
assert.match(
report,
/\| `react-router\.full` \| 1\.17 KiB \| \+100 B \(\+9\.09%\) \|/,
)
})

test('keeps scenarios that do not have baseline data visible', async () => {
const current = currentJson([
metric('react-router.minimal', 1_000),
metric('new-scenario', 900),
])
const baseline = currentJson([metric('react-router.minimal', 1_000)])
const report = await generateReport({ current, baseline })

assert.match(
report,
/The following scenarios have bundle-size changes or lack baseline data for comparison:/,
)
assert.doesNotMatch(report, /`react-router\.minimal`/)
assert.match(report, /\| `new-scenario` \| 900 B \| n\/a \|/)
})
Loading