fix(scripts): pin the catalog generation locale so assets are reproducible - #1129
Open
sukvvon wants to merge 1 commit into
Open
fix(scripts): pin the catalog generation locale so assets are reproducible#1129sukvvon wants to merge 1 commit into
sukvvon wants to merge 1 commit into
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tanstack-com | c1160d3 | Commit Preview URL Branch Preview URL |
Aug 08 2026, 03:09 AM |
📝 WalkthroughWalkthroughThe chart catalog and asset generation scripts now load a shared locale shim. The shim defaults omitted date and number formatting locales to ChangesCatalog locale determinism
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@scripts/pin-catalog-locale.ts`:
- Around line 25-29: Update all three locale wrapper methods in
scripts/pin-catalog-locale.ts to default only when locales is undefined,
preserving explicit null and other invalid values for native validation. Replace
the nullish fallback in each originalToLocaleDateString call with an
undefined-only conditional while keeping the existing GENERATION_LOCALE default
and call flow unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a07e3ab6-8fdf-40e5-9b53-808ec6b3aa48
📒 Files selected for processing (3)
scripts/generate-charts-landing-assets.tsscripts/generate-charts-landing-catalog.tsscripts/pin-catalog-locale.ts
Comment on lines
+25
to
+29
| return originalToLocaleDateString.call( | ||
| this, | ||
| locales ?? GENERATION_LOCALE, | ||
| options, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file outline ---\n'
ast-grep outline scripts/pin-catalog-locale.ts --view expanded || true
printf '\n--- relevant file contents ---\n'
cat -n scripts/pin-catalog-locale.ts
printf '\n--- native behavioral probe ---\n'
node - <<'JS'
const dates = [
new Date('2026-08-20T12:34:56Z'),
new Date('invalid date')
];
const options = {};
for (const date of dates) {
for (const fn of ['toLocaleDateString','toLocaleTimeString','toLocaleString']) {
const original = Date.prototype[fn] || Object.getPrototypeOf(new Date())?.[fn];
console.log(original, fn, date.toISOString());
for (const input of [undefined, null, 'en-US', 'invalid-locale']) {
try {
const result = original.call(date, input, options);
console.log(`${fn}(${JSON.stringify(input)}) -> ${result}`);
} catch (error) {
console.log(`${fn}(${JSON.stringify(input)}) throws ${error.name}: ${error.message}`);
}
}
}
}
JS
printf '\n--- search for generated default / helpers usages ---\n'
rg -n "GENERATION_LOCALE|pin-catalog|Catalog|locale" scripts package.json -S || trueRepository: TanStack/tanstack.com
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- file outline ---'
ast-grep outline scripts/pin-catalog-locale.ts --view expanded || true
printf '%s\n' ''
printf '%s\n' '--- relevant file contents ---'
cat -n scripts/pin-catalog-locale.ts
printf '%s\n' ''
printf '%s\n' '--- native behavioral probe ---'
node - <<'JS'
const dates = [new Date('2026-08-20T12:34:56Z'), new Date('invalid date')];
const options = {};
for (const date of dates) {
for (const fn of ['toLocaleDateString','toLocaleTimeString','toLocaleString']) {
const dateProto = Object.getPrototypeOf(new Date());
const original = dateProto[fn];
console.log(dateProto, fn, date.toISOString());
for (const input of [undefined, null, 'en-US', 'invalid-locale']) {
try {
const result = original.call(date, input, options);
console.log(`${fn}(${JSON.stringify(input)}) -> ${result}`);
} catch (error) {
console.log(`${fn}(${JSON.stringify(input)}) throws ${error.name}: ${error.message}`);
}
}
}
}
JS
printf '%s\n' ''
printf '%s\n' '--- search for generated default / helpers usages ---'
rg -n "GENERATION_LOCALE|pin-catalog|Catalog|locale" scripts package.json package-lock.json bun.lockb pnpm-lock.yaml yarn.lock -S || trueRepository: TanStack/tanstack.com
Length of output: 4352
Preserve explicit invalid locale values.
locales ?? GENERATION_LOCALE treats null as omitted and formats it as en-US, while the native methods throw. Use locales === undefined ? GENERATION_LOCALE : locales in all three wrappers so only omitted locale arguments receive the generation default.
🤖 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/pin-catalog-locale.ts` around lines 25 - 29, Update all three locale
wrapper methods in scripts/pin-catalog-locale.ts to default only when locales is
undefined, preserving explicit null and other invalid values for native
validation. Replace the nullish fallback in each originalToLocaleDateString call
with an undefined-only conditional while keeping the existing GENERATION_LOCALE
default and call flow unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pnpm testfails for anyone whose machine is not set to an English locale. Thepretesthook runscharts:check-landing-catalog, which regenerates the catalog assets in memory and compares them against what is committed — and the regenerated SVGs differ, so the check reports the committed assets as stale.The difference is the x-axis date labels. Three cases render
Oct 6,Oct 8,Oct 10,Oct 12on an English machine and10월 6일,10월 8일, … on a Korean one:86-streaming-window-preservation,91-timeline-playback-scrubber,92-editable-event-range. That text is part of the SVG, so it changes the content hash, which changeschartsLandingCatalogAssetRevision, which fails the check.Where it comes from
The formatting happens inside
@tanstack/react-charts-catalog(0.7.2), not in this repo. Its catalog cases pinen-USin 96 places and leave 29toLocaleDateString/toLocaleStringcalls without a locale, so those fall back to the machine's. The cases already fix their data and passtimeZone: 'UTC', so the missing locale reads as an oversight rather than a decision.The real fix is TanStack/charts#68, which pins those 29 call sites. This PR is the guard until that lands, is published, and the version is bumped here — three separate events, and the check stays broken locally in the meantime. Once the bump happens, drop the two
import './pin-catalog-locale'lines and re-run the check under a non-English locale; if it passes,scripts/pin-catalog-locale.tscan be deleted.The change
scripts/pin-catalog-locale.tsdefaultsDate.prototype.toLocaleDateString,Date.prototype.toLocaleString, andNumber.prototype.toLocaleStringtoen-US, and both generation scripts import it for side effects before the catalog components load. Calls that pass a locale explicitly keep it — the wrapper islocales ?? 'en-US', so the 32 already-pinned call sites are untouched.Setting
process.env.LANGinside the script does not work: Node resolves ICU's default locale at process start, before any module code runs. Pinning in only one script is also not enough, since the pre-commit chain isgenerate-charts-landing-catalog --check && generate-charts-landing-assets --checkand both load the case components.Verification
Under
LANG=ko_KR.UTF-8, regenerating now produces zero changed files — previously three — andchartsLandingCatalogAssetRevisionstays at487bba6af954. The generated assets are byte-identical to what is committed, so nothing about the rendered charts changes.Both
--checkscripts pass underko_KR.UTF-8and underen_US.UTF-8. This commit was made with the pre-commit hook enabled on a Korean-locale machine, which previously required--no-verify.Explicit locales still work: with the patch loaded,
toLocaleDateString('ko-KR', …)returns10월 6일,('ja-JP', …)returns10月6日, and(1234.5).toLocaleString('de-DE')returns1.234,5.Scope is limited to asset generation. The scripts run as separate
tsxprocesses and nothing in the app bundle imports this module, so runtime rendering — includingcharts.catalog_.embed.$caseId.tsx, which renders the same case components live — is unaffected and still follows the visitor's locale.Summary by CodeRabbit