Skip to content

perf(i18n): replace regex in t() with replaceAll#2176

Merged
esengine merged 1 commit into
esengine:mainfrom
HUQIANTAO:perf/config-cache-and-i18n
May 29, 2026
Merged

perf(i18n): replace regex in t() with replaceAll#2176
esengine merged 1 commit into
esengine:mainfrom
HUQIANTAO:perf/config-cache-and-i18n

Conversation

@HUQIANTAO

@HUQIANTAO HUQIANTAO commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Replace per-key new RegExp(\\{${k}\}`, "g")with nativereplaceAll(`{${k}}`, String(v))in thet()` translation function — avoids regex construction overhead on every translation lookup.

The readConfig mtime cache that was originally in this PR has been dropped — it's already in #2162 (which uses an fd-based approach to avoid a TOCTOU race flagged by CodeQL).

Files changed (1)

File Change
src/i18n/index.ts result.replace(new RegExp(...))result.replaceAll(...)

Test plan

  • t("key", { name: "value" }) still substitutes {name} correctly
  • Multiple params in one string all substitute

🤖 Generated with Claude Code

@esengine

Copy link
Copy Markdown
Owner

Heads up — #2162 just merged and already includes a readConfig mtime cache, so that half of this PR is now redundant. Importantly, #2162's version reads through an open file descriptor (openSyncfstatSync(fd)readFileSync(fd)) to avoid a TOCTOU race — CodeQL flagged the plain statSync(path)-then-readFileSync(path) pattern (which is what this PR uses) as a high-severity file race. So please drop the readConfig hunk here (it'd reintroduce the alert and conflicts with main).

The t() replaceAll change is genuinely useful and not in #2162 — please rebase and reduce this PR to just that one-liner (replacing the per-key new RegExp(...,'g') with replaceAll). Small, clean, happy to merge that on its own.

Replace per-key `new RegExp(\`\\{${k}\\}\`, "g")` with native
`replaceAll(\`{${k}}\`, String(v))` — avoids regex construction overhead
on every translation lookup. ReadConfig cache already merged in esengine#2162.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@HUQIANTAO HUQIANTAO force-pushed the perf/config-cache-and-i18n branch from 8886195 to 7262a4e Compare May 29, 2026 04:43
@HUQIANTAO HUQIANTAO changed the title perf: cache readConfig by mtime; replace regex in t() with replaceAll perf(i18n): replace regex in t() with replaceAll May 29, 2026

@esengine esengine left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed — trimmed exactly as discussed: now just src/i18n/index.ts, dropping the redundant (and TOCTOU-prone) readConfig cache that #2162 already covers, keeping only the t() win — result.replaceAll(\{${k}}`, String(v))instead ofnew RegExp(`\{${k}\}`, "g")` per key. Equivalent (literal global replace) and avoids constructing a RegExp per placeholder per call. CI green. Merging.

@esengine esengine merged commit 88114f2 into esengine:main May 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants