Conversation
Babel's typescript plugin was gated on the config file name. A TypeScript
config evaluated under a .js or .mjs name is therefore parsed as JavaScript,
and any TS-only syntax fails.
Production 2026-08-06: a customer's veryfront.config.ts containing
`router: "pages" as const` returned HTTP 400 'Hosted configuration
rejected (syntax-error: syntax-error)' and took their site down.
Reproduced against the real config through evaluateHostedConfigSource, the
same seam a hosted request uses:
before veryfront.config.ts parses
veryfront.config.js config-parse-error 400 syntax-error
veryfront.config.mjs config-parse-error 400 syntax-error
after all three parse
The 400 is byte-identical to production, and only a non-.ts name produces
it. Which seam supplies the non-.ts name is not yet pinned; the release
asset path is enabled in production and carries its own fileName. That
question is worth answering separately, but it does not change this fix:
the parser should not infer language from a name it cannot trust.
Latent behind the missing worker entry fixed in #3399 — that bug crashed
the evaluator before it could parse anything, so the hosted declarative
path had never run against a real project config in production.
TypeScript is a superset, so enabling it unconditionally cannot reject
valid JavaScript. The existing `!filePath` branch already relied on that;
this makes the behaviour uniform rather than adding a new one.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Babel parser now accepts TypeScript syntax for all supported configuration filenames. Regression tests cover TypeScript and JavaScript configurations. The project version changed from ChangesBabel parser behavior
Project version update
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Is "always TypeScript" actually safe? Measured, not assumed.Fair question on review, and "TypeScript is a superset" is a claim worth testing rather than asserting — the real risk is that Same constructs, parsed under
No difference on any of them, including the ambiguous ones. The What this does not fixThis makes the parser stop guessing language from a name it cannot trust. It does not explain why a
So I would land this as hardening, on the grounds that a parser should not infer language from an untrusted name, and track the mislabel separately rather than let the disappearing 400 close the question. Verified: |
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 `@extensions/ext-parser-babel/src/parser-only.test.ts`:
- Around line 140-160: Update the assertion import in parser-only.test.ts to use
the repository helper from `#veryfront/testing/assert.ts` instead of `@std/assert`,
while preserving the existing assert calls in the TypeScript parsing tests.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 91db55b0-48b9-4562-8bf4-54fc865a6b3f
📒 Files selected for processing (4)
deno.jsonextensions/ext-parser-babel/src/parser-only.test.tsextensions/ext-parser-babel/src/parser-only.tssrc/utils/version-constant.ts
| it("parses TypeScript syntax under a .js file name", async () => { | ||
| // The hosted config loader cannot trust the extension. Its file endpoint is | ||
| // extension-tolerant, so a request for veryfront.config.js is answered with | ||
| // the project's veryfront.config.ts content, and the loader reports the | ||
| // candidate it asked for. Gating the TypeScript plugin on that name parsed | ||
| // TypeScript as JavaScript and rejected every TS config as a syntax error. | ||
| // | ||
| // Production 2026-08-06: `router: "pages" as const` in a customer's | ||
| // veryfront.config.ts returned HTTP 400 "Hosted configuration rejected | ||
| // (syntax-error: syntax-error)" and took their site down. | ||
| const source = [ | ||
| "export default {", | ||
| ' router: "pages" as const,', | ||
| ' app: "components/app.tsx",', | ||
| "};", | ||
| ].join("\n"); | ||
|
|
||
| for (const filePath of ["veryfront.config.js", "veryfront.config.mjs", "veryfront.config.ts"]) { | ||
| const ast = await parser.parse({ code: source, filePath }); | ||
| assert(ast, `expected ${filePath} to parse TypeScript syntax`); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the repository assertion helper.
The new tests call assert, but parser-only.test.ts imports it from @std/assert on Line 1. The repository guideline requires assertions from #veryfront/testing/assert.ts. Update the assertion import before merge.
Also applies to: 163-175
🤖 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 `@extensions/ext-parser-babel/src/parser-only.test.ts` around lines 140 - 160,
Update the assertion import in parser-only.test.ts to use the repository helper
from `#veryfront/testing/assert.ts` instead of `@std/assert`, while preserving the
existing assert calls in the TypeScript parsing tests.
Source: Coding guidelines
The parser-level test proves the plugin selection. This one reproduces the actual production failure signature end to end: the customer's config, byte for byte, evaluated through evaluateDeclarativeConfig under each recognized file name. Against the shipped parser it fails with the production string verbatim: DeclarativeConfigEvaluationError: Hosted configuration rejected (syntax-error: syntax-error) With the fix all three names evaluate and router resolves to "pages".
Restores a customer site that is returning HTTP 400 in production right now.
Symptom
The config is valid TypeScript:
Cause
pickPluginsselected Babel'stypescriptplugin from the file name:Under a
.jsor.mjsname a TypeScript config is parsed as JavaScript.as constfails,parserErrorReasonmatches neitherDuplicateDefaultExportnorVarRedeclarationand falls through to the generic"syntax-error", which surfaces asCONFIG_PARSE_ERROR→ HTTP 400.The evaluator cannot trust that name. It is the candidate the hosted loader asked for, not necessarily the file it received.
Reproduced locally, then fixed
Both reproductions are committed as tests, and both fail on
main.At the evaluator boundary —
declarative-evaluator.test.ts, the customer's config byte for byte throughevaluateDeclarativeConfig:That error string is verbatim what production returns.
At the parser —
parser-only.test.ts:Column 18 is exactly where
as constbegins.Does "always TypeScript" break JavaScript?
Measured rather than assumed, since
typescript+jsxputs.jsinto TSX mode where<can read as type arguments instead of comparison. Same constructs underveryfront.config.js, before and after:a < b > (c)f < g > (h)/a<b>/g.js#p,static), dynamicimport()No difference on any of them, including the ambiguous ones. TypeScript is a superset, so enabling it cannot reject valid JavaScript — it only widens what parses. The pre-existing
!filePathbranch already depended on that; this makes the behaviour uniform rather than introducing a new one.Blast radius is small:
parser-only.tshas two consumers, both the declarative config evaluator.What this does not fix
Why a
.tsconfig is labelled.js. This stops the parser guessing language from an untrusted name; it does not explain the mislabel. The runtime/files/endpoint is not extension-tolerant —veryfront.config.jsgenuinely 404s for this release while.tsreturns the content — so it is not simply the loader's candidate order.VERYFRONT_RELEASE_ASSET_MANIFESTis enabled in production and that seam carries its ownfileName; that is where I would look next.This matters beyond parsing:
DeclarativeConfigFileNamealso feedsconfigFileProvenance, parse-error locations, and the hosted config cache key. If the release-asset path records.jsfor a.tsfile, that is a data-integrity bug which will resurface somewhere less obvious — and this PR makes it silent rather than loud. Worth tracking separately rather than letting the disappearing 400 close the question.tomcode.com, also down, with a different defect:
404 API request failed. Its release has noveryfront.config.*at all, which should hit the loader's own"No config file found, using defaults"fallback. It does not, because the API-backed filesystem reports a missing file asAPI_CLIENT_ERRORwithstatus: 404, whileisNotFoundErrormatches onlyENOENT/ENOTDIR, Deno'sNotFound, or slugfile-not-found. The 404 propagates instead of being read as absence. The fix belongs in the fs adapter — translate the transport 404 into the platform not-found error — not in the classifier, which should not sniff HTTP status or every API failure starts looking like an absent file.Both defects were latent behind the missing worker entry fixed in #3399: that bug crashed the evaluator before it parsed anything, so this path had never run against a real project config in production.
Tests
src/config/— 44 passed, 402 steps.extensions/ext-parser-babel/— 3 passed, 36 steps.Summary by CodeRabbit
Bug Fixes
.js,.mjs, and.tsconfiguration files.as const.Chores