Conversation
Standard Schema
|
Caution Review failedThe pull request is closed. WalkthroughAdds a standard-schema validator path and metadata-driven query parsing, converts macros to declarative hook configs with standalone validators, replaces public Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as Router
participant H as Handler
participant V as Validator
participant S as Status/Response
C->>R: HTTP Request (GET/POST/HEAD)
alt HEAD & web-standard
R->>S: compute Content-Length via getResponseLength
S-->>C: 200 + Content-Length
else
R->>V: run validators from route.hooks.standaloneValidator
alt provider == "standard"
V->>V: standard.validate (maybe async)
else
V->>V: typebox Check/Decode/Parse
end
alt validation issues
V-->>R: ValidationError -> 422
R->>C: 422 with issues
else
R->>H: invoke handler with context (includes status function)
alt handler uses status(...)
H-->>R: status response (custom code/payload)
else
H-->>R: returns value -> response schema applied
end
R->>C: HTTP response
end
end
sequenceDiagram
autonumber
participant P as Plugin/Builder
participant U as insertStandaloneValidator
participant H as Route.hooks
P->>U: add params/query/body validators
U->>H: push validators into hooks.standaloneValidator[]
Note right of H: compose/adapter/validator read hooks.standaloneValidator to perform coercion & parsing
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing touches
🧪 Generate unit tests
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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
test/aot/analysis.test.ts (1)
133-133: Missing matcher invocation: call toBeTrue().Currently the assertion is not executed.
Apply this diff:
- expect(hasType('Numeric', schema)).toBeTrue + expect(hasType('Numeric', schema)).toBeTrue()src/cookies.ts (2)
354-367: Signature-rotation bug: decoded flag initialized incorrectly (never throws)decoded is set to true before verification, so invalid signatures won’t throw. Initialize to false and only set true upon success.
Apply this diff:
- let decoded = true + let decoded = false for (let i = 0; i < secrets.length; i++) { const temp = await unsignCookie(value as string, secrets[i]) if (temp !== false) { - decoded = true + decoded = true value = temp break } } if (!decoded) throw new InvalidCookieSignature(name)
344-373: Signed JSON values aren’t parsed back to objects after unsignCurrently JSON parsing happens only before unsigning; signed JSON ends up as a string. Re-run the lightweight JSON check after successful unsign to keep behavior consistent between signed/unsigned cookies.
Apply this diff (insert before jar assignment):
} - jar[name] = { + // Parse JSON-like string after possible unsign as well + if (typeof value === 'string' && value.length >= 2) { + const starts = value.charCodeAt(0) + const ends = value.charCodeAt(value.length - 1) + if ( + (starts === 123 && ends === 125) || + (starts === 91 && ends === 93) + ) { + try { + value = JSON.parse(value) + } catch {} + } + } + jar[name] = { value }test/core/handle-error.test.ts (1)
155-164: Fix duplicate test: actually throw a native Response“handle thrown Response” currently throws status(...) again, duplicating the previous test. Throw a Response to cover that path.
Apply:
- const app = new Elysia().get('/', ({ status }) => { - throw status(404, 'Not Found :(') - }) + const app = new Elysia().get('/', () => { + throw new Response('Not Found :(', { status: 404 }) + })test/cookie/response.test.ts (1)
350-361: Await the headers Promise to avoid a false negativeres is a Promise; expect(res).toEqual({}) will fail or pass incorrectly under TS suppression. Await it.
Apply:
- const res = app.handle(req('/')).then((x) => x.headers.toJSON()) - - // @ts-expect-error - expect(res).toEqual({}) + const res = await app.handle(req('/')).then((x) => x.headers.toJSON()) + expect(res).toEqual({})src/type-system/utils.ts (1)
128-155: Bug: string extension falls through and is iterated by character.If
extensionis a string and the initial check passes, control still falls through to the loop and compares MIME against individual characters, leading to false throws.Apply:
-export const fileType = async ( - file: MaybeArray<Blob | File | undefined>, - extension: FileType | FileType[], - // @ts-ignore - name = file?.name ?? '' -): Promise<boolean> => { +export const fileType = async ( + file: MaybeArray<Blob | File | undefined>, + extension: FileType | FileType[], + name?: string +): Promise<boolean> => { if (Array.isArray(file)) { - await Promise.all( - file.map((f) => fileType(f, extension, name)) - ) + await Promise.all(file.map((f) => fileType(f, extension, name ?? (f as any)?.name ?? ''))) return true } if (!file) return false const result = await fileTypeFromBlob(file) - if (!result) throw new InvalidFileType(name, extension) + if (!result) throw new InvalidFileType(name ?? (file as any)?.name ?? '', extension as any) - if (typeof extension === 'string') - if (!checkFileExtension(result.mime, extension)) - throw new InvalidFileType(name, extension) + const exts = Array.isArray(extension) ? extension : [extension as string] - for (let i = 0; i < extension.length; i++) - if (checkFileExtension(result.mime, extension[i])) return true + for (let i = 0; i < exts.length; i++) + if (checkFileExtension(result.mime, exts[i] as string)) return true - throw new InvalidFileType(name, extension) + throw new InvalidFileType(name ?? (file as any)?.name ?? '', exts as any) }src/error.ts (1)
452-459: HTTP status mismatch (400 vs class status 422).Return
this.statusto align withValidationError.status = 422and test expectations.- return new Response(this.message, { - status: 400, + return new Response(this.message, { + status: this.status,src/parse-query.ts (1)
24-35: Honor startIndex in the scan.Currently scans from 0 and relies on
startingIndex = startIndex - 1, which can mis-handle=beforestartIndex.- for (let i = 0; i < inputLength; i++) + for (let i = startIndex; i < inputLength; i++)test/macro/macro.test.ts (1)
354-376: Test has no assertion — it can’t fail even if behavior regressesThe test named “don't duplicate call on as plugin” never asserts
called. Add an assertion to verify no duplicate invocation.Apply:
await app.handle(req('/')) + + expect(called).toBe(1)If you intended to also exercise the app route (
/foo), add another request and adjust the expectation accordingly.src/schema.ts (1)
113-114: Premature return in hasAdditionalProperties()Returning inside the first property iteration stops traversal and yields incorrect results.
Apply:
- return property.additionalProperties + // Do not return here; continue scanning nested structures + // additionalProperties is only meaningful on object schemas + continue
🧹 Nitpick comments (54)
test/type-system/uint8array.test.ts (1)
71-79: Strengthen the test with status and byte-accurate assertionsAdd checks for HTTP 200 and byte-length integrity to better guard against regressions in binary handling.
- const response = await app.handle( - new Request('http://localhost', { - method: 'POST', - body: new TextEncoder().encode('可愛くてごめん'), - headers: { 'content-type': 'application/octet-stream' } - }) - ) + const payload = new TextEncoder().encode('可愛くてごめん') + const response = await app.handle( + new Request('http://localhost', { + method: 'POST', + body: payload, + headers: { 'content-type': 'application/octet-stream' } + }) + ) - expect(await response.text()).toBe('可愛くてごめん') + expect(response.status).toBe(200) + const buf = await response.arrayBuffer() + expect(buf.byteLength).toBe(payload.byteLength) + expect(new TextDecoder().decode(buf)).toBe('可愛くてごめん') + expect(await response.text()).toBe('可愛くてごめん')test/adapter/web-standard/map-response.test.ts (1)
275-283: Remove// @ts-ignore; use a type-safe headers snapshot instead of non-standardHeaders.toJSON().Avoid suppressing type errors. Use
Object.fromEntries(headers)which is standard and typed, while still normalizing header names to lowercase.- const headers = response.headers.toJSON() + const headers = Object.fromEntries(response.headers) @@ - // @ts-ignore - expect(response.headers.toJSON()).toEqual({ + expect(Object.fromEntries(response.headers)).toEqual({ ...headers, name: 'Himari' })test/adapter/web-standard/map-early-response.test.ts (1)
234-242: Replace// @ts-ignorewith a standard, typed header serialization.Rely on
Object.fromEntriesover the Bun-specifictoJSON()to keep tests portable and type-safe. Use!since you already assertedresponseis aResponse.- const headers = response?.headers.toJSON() + const headers = Object.fromEntries(response!.headers) @@ - // @ts-ignore - expect(response?.headers.toJSON()).toEqual({ + expect(Object.fromEntries(response!.headers)).toEqual({ ...headers, name: 'Himari' })example/a.ts (1)
4-19: Optional: Clarify input and status semantics for a POST /user.If this endpoint creates a resource, consider returning 201 and setting the status explicitly; this also makes the example of per-status schemas clearer.
Example adjustment:
- ({ q }) => { - return { q: 'a' } + ({ set }) => { + set.status = 201 + return { name: 'a' } }, { response: { - 200: t.Object({ + 201: t.Object({ name: t.String() }), q: t.String() }) } }test/type-system/union-enum.test.ts (1)
16-27: Add explicit negative check for null to prevent regressionWe’ve removed null from the union; let’s assert it stays rejected.
it('Check', () => { const schema = t.UnionEnum(['some', 'data']) expect(Value.Check(schema, 'some')).toBe(true) expect(Value.Check(schema, 'data')).toBe(true) expect(Value.Check(schema, { deep: 2 })).toBe(false) expect(Value.Check(schema, 'yay')).toBe(false) expect(Value.Check(schema, 42)).toBe(false) expect(Value.Check(schema, {})).toBe(false) + expect(Value.Check(schema, null)).toBe(false) expect(Value.Check(schema, undefined)).toBe(false) })test/sucrose/infer-body-reference.test.ts (7)
2-2: Prefer type-only import for Sucrose.Avoids emitting a runtime import if Sucrose is type-only in some builds.
-import { inferBodyReference, Sucrose } from '../../src/sucrose' +import { inferBodyReference } from '../../src/sucrose' +import type { Sucrose } from '../../src/sucrose'
8-19: Widen the inference object to boolean to drop casts.Using satisfies keeps literal false types, forcing casts in expectations. Annotate as Sucrose.Inference instead.
-const inference = { +const inference: Sucrose.Inference = { query: false, headers: false, body: false, cookie: false, set: false, server: false, path: false, route: false, url: false -} satisfies Sucrose.Inference +}And remove the unnecessary cast:
-expect(inference.body as boolean).toBe(true) +expect(inference.body).toBe(true)Also applies to: 22-22
28-39: Apply the same typing tweak here.Widen to Sucrose.Inference and drop the cast.
-const inference = { +const inference: Sucrose.Inference = { query: false, headers: false, body: false, cookie: false, set: false, server: false, path: false, route: false, url: false -} satisfies Sucrose.Inference +}-expect(inference.body as boolean).toBe(true) +expect(inference.body).toBe(true)Also applies to: 42-42
61-65: Consistent typing for inference.Match other blocks: annotate instead of using satisfies.
-const inference = { +const inference: Sucrose.Inference = { body: false, cookie: false, headers: false, query: true, set: true, server: false, path: false, route: false, url: false -} satisfies Sucrose.Inference +}
156-160: Consistent typing for inference (dynamic quote case).-const inference = { +const inference: Sucrose.Inference = { body: false, cookie: false, headers: false, query: true, set: true, server: false, path: false, route: false, url: false -} satisfies Sucrose.Inference +}
182-193: Same typing cleanup + remove cast; consider adding tests for route/url/path.Annotate as Sucrose.Inference and drop cast. Also, since server is now covered, adding small tests for route/url/path would fully exercise the new flags.
-const inference = { +const inference: Sucrose.Inference = { query: false, headers: false, body: false, cookie: false, set: false, server: false, path: false, route: false, url: false -} satisfies Sucrose.Inference +}-expect(inference.server as boolean).toBe(true) +expect(inference.server).toBe(true)Example follow-ups (sketch):
- url: code = 'context.url.pathname'
- route: code = 'context.route.name'
- path: code = 'context.path'
Also applies to: 196-196
177-177: Disambiguate duplicate test titles.There are two “infer dot notation” tests; rename this one for clarity.
-it('infer dot notation', () => { +it('infer dot notation: server access', () => {test/lifecycle/parser.test.ts (1)
431-433: Replace ts-ignore with a typed narrow (and consider fixing the onError type).Avoid blanket
@ts-ignore; narrow the shape instead. Ifctx.codeis intended, expose it in the onError context type; meanwhile, narrow locally.- // @ts-ignore - code = ctx.code + code = (ctx as { code?: string }).codetest/core/native-static.test.ts (1)
1-1: Don’t disable type-checking for the whole file.Prefer targeted casts or
@ts-expect-errorat the specific assertions that accessapp.router.response. This keeps the rest of the file type-safe.Example localized cast (no need to change runtime):
const root = app.router.response['/'] as { GET: Response } expect(root.GET).toBeInstanceOf(Response)src/type-system/types.ts (1)
19-19: Unused import: ElysiaFile.This import isn’t referenced. Either remove it, or update
TFile/TFilesto useElysiaFileif that’s the intended unification.Option A (remove):
-import { ElysiaFile } from '../universal/file'Option B (adopt):
-export type TFile = ( - options?: Partial<FileOptions> | undefined -) => TUnsafe<File> +export type TFile = ( + options?: Partial<FileOptions> | undefined +) => TUnsafe<ElysiaFile> -export type TFiles = ( - options?: Partial<FilesOptions> | undefined -) => TUnsafe<File[]> +export type TFiles = ( + options?: Partial<FilesOptions> | undefined +) => TUnsafe<ElysiaFile[]>example/websocket.ts (1)
15-15: Confirm intent: echo + broadcast now sends twice.
ws.publish('asdf', message)broadcasts to subscribers;ws.send(message)echoes back to the sender. If duplication isn’t intended, drop one.- ws.publish('asdf', message) - ws.send(message) + // EITHER broadcast: + ws.publish('asdf', message) + // OR echo: + // ws.send(message)src/adapter/bun/index.ts (1)
488-488: Avoidas anyby casting to the function’s parameter type.This retains safety without importing extra types.
- const parseMessage = createWSMessageParser(parse as any) + const parseMessage = createWSMessageParser( + parse as Parameters<typeof createWSMessageParser>[0] + )test/lifecycle/error.test.ts (1)
129-131: Consistent switch tostatus(...)All four cases correctly cover number and named statuses, with/without body, under both AOT modes.
- Consider renaming test titles from “on error function” to “on status function” for clarity.
Also applies to: 142-144, 155-156, 167-169
test/lifecycle/transform.test.ts (2)
44-51: Avoid@ts-ignoreby typing the group transformYou can type
paramsin the group’sonTransformto eliminate the suppression.- .group('/scoped/id/:id', (app) => - app - .onTransform(({ params }) => { - // @ts-ignore - if (params.id) params.id = +params.id - }) - .get('', ({ params: { id } }) => typeof id) - ) + .group('/scoped/id/:id', (app) => + app + .onTransform<{ params: { id: number | string } | null }>(({ params }) => { + if (params?.id != null) params.id = +params.id as any + }) + .get('', ({ params: { id } }) => typeof id) + )
70-71: Remove@ts-ignorein plugin transformType the
onTransformgeneric to reflect pre-transform shape.->({ as: 'global' }, (request) => { - // @ts-ignore - if (request.params?.id) request.params.id = +request.params.id -}) +>({ as: 'global' }, <T extends { params: { id: number | string } | null }>(request: T) => { + if (request.params?.id != null) (request.params as any).id = +request.params.id +})test/validator/query.test.ts (1)
970-981: Good coverage for validation error capture via route-level error hookThis verifies VALIDATION errors surface as ValidationError. Consider also asserting error.code == 'VALIDATION' for completeness, but current test is fine.
src/ws/types.ts (1)
135-136: Tighten upgrade() context typing to route schema/singletonupgrade currently receives Context (defaulted). Binding it to the WS route’s Schema/Singleton improves inference without breaking changes.
Apply this diff:
- upgrade?: Record<string, unknown> | ((context: Context) => unknown) + upgrade?: Record<string, unknown> | (( + context: Context<Schema, Singleton> + ) => unknown)test/validator/response.test.ts (1)
298-307: Rename test title for clarityThe test uses status(), but the title still says “with error()”. Suggest renaming to reflect status-based flow.
Apply this diff:
- it('validate response per status with error()', async () => { + it('validate response per status with status()', async () => {src/cookies.ts (2)
331-343: Guard string operations; avoid charCodeAt on non-stringIf value becomes non-string earlier (future changes), charCodeAt would throw. Minor, but cheap to guard.
Apply this diff:
- if (value) { + if (typeof value === 'string' && value) { const starts = value.charCodeAt(0) const ends = value.charCodeAt(value.length - 1) if ( (starts === 123 && ends === 125) || (starts === 91 && ends === 93) ) try { value = JSON.parse(value) } catch {} }
307-376: Consider gating JSON auto-parse behind an optionAuto-parsing any {…} or […] value may surprise apps expecting raw strings. If this is a breaking change, consider an opt-in flag (e.g., parseJsonCookie?: boolean) defaulting to current behavior, or document the change prominently in 1.4 release notes.
test/core/elysia.test.ts (2)
391-404: Make HEAD header assertion resilientComparing the entire headers object is brittle across runtimes. Assert only Content-Length.
Apply this diff:
- expect(response.status).toBe(200) - expect(response.headers.toJSON()).toEqual({ - 'content-length': '11', - }) + expect(response.status).toBe(200) + expect(response.headers.get('content-length')).toBe('11')
406-419: Same for dynamic path HEADLimit the expectation to Content-Length to avoid incidental header diffs.
Apply this diff:
- expect(response.status).toBe(200) - expect(response.headers.toJSON()).toEqual({ - 'content-length': '11', - }) + expect(response.status).toBe(200) + expect(response.headers.get('content-length')).toBe('11')test/lifecycle/map-response.test.ts (1)
246-246: Avoid ad-hoc string coercion in mapResponseUse String(...) to coerce safely; handles undefined without concatenation side effects.
Apply this diff:
- return new Response(context.response + '') + return new Response(String(context.response ?? ''))test/core/normalize.test.ts (4)
78-80: Prefer ts-expect-error over ts-ignoreAssert the intended type error rather than silencing it.
Apply this diff:
- // @ts-ignore + // @ts-expect-error
92-96: Also assert status code for the 418 branchHelps ensure status(...) is preserved at runtime.
Apply this diff:
- const response = await app.handle(req('/')).then((x) => x.json()) + const res = await app.handle(req('/')) + expect(res.status).toBe(418) + const response = await res.json()
104-106: Same note on ts-expect-errorKeep the test intention explicit.
Apply this diff:
- // @ts-ignore + // @ts-expect-error
118-121: Add status assertion here as wellMirror the 418 status check for the non-normalized path.
Apply this diff:
- const response = await app.handle(req('/')) - - expect(response.status).toEqual(422) + const response = await app.handle(req('/')) + expect(response.status).toEqual(422)Optional: keep as-is if you prefer minimal changes; above ensures consistency with the previous test.
src/adapter/bun/compose.ts (2)
41-43: Guard InputSchema check and prefer.somefor intent clarityUsing
.find((x) => x.query)works, but.some((x) => !!x?.query)communicates boolean intent and safely handles sparse entries.Apply:
- !!(route.hooks.standaloneValidator as InputSchema[])?.find( - (x) => x.query - ) || + !!(route.hooks.standaloneValidator as InputSchema[])?.some( + (x) => !!x?.query + ) ||
38-46: De-duplicate needsQuery computationThe same needsQuery logic appears in createContext and createBunRouteHandler. Centralize it to avoid drift.
Example:
+const routeNeedsQuery = (route: InternalRoute, inference: Sucrose.Inference, app: AnyElysia) => + inference.query || + !!route.hooks.query || + !!(route.hooks.standaloneValidator as InputSchema[])?.some((x) => !!x?.query) || + app.event.request?.length; ... - const needsQuery = - inference.query || - !!route.hooks.query || - !!(route.hooks.standaloneValidator as InputSchema[])?.some((x) => !!x?.query) || - app.event.request?.length + const needsQuery = routeNeedsQuery(route, inference, app) ... - const needsQuery = - inference.query || - !!route.hooks.query || - !!(route.hooks.standaloneValidator as InputSchema[])?.some((x) => !!x?.query) + const needsQuery = routeNeedsQuery(route, inference, app)Also applies to: 132-141
test/core/handle-error.test.ts (1)
251-269: Use numeric code for clarity (optional)status("I'm a teapot", ...) depends on name-to-code mapping. Consider status(418, ...) to avoid ambiguity.
- throw status("I'm a teapot", { message: 'meow!' }) + throw status(418, { message: 'meow!' })And:
- status(422, value) as any + status(422, value) as any(unchanged, included for context)
test/types/lifecycle/soundness.ts (2)
1-4: Drop unused import to keep TS tidyElysiaCustomStatusResponse isn’t referenced; remove it to avoid noUnusedLocals in stricter setups.
-import { Cookie, Elysia, ElysiaCustomStatusResponse, t } from '../../../src' +import { Cookie, Elysia, t } from '../../../src'
1534-1536: Remove unused type aliasestype A aliases are unused and may trip noUnusedLocals. Safe to remove.
- type A = keyof (typeof app)['~Routes']['get']['response']Also applies to: 1601-1603
test/standard-schema/validate.test.ts (1)
155-175: Reduce repetition with table-driven helpers.The repeated “requests -> status list” pattern can be factored into a small helper to keep tests terse and easier to extend.
-const responses = await Promise.all([...].map((x) => app.handle(x).then((x) => x.status))) +const statuses = (reqs: Request[]) => Promise.all(reqs.map(r => app.handle(r).then(r => r.status))) +const responses = await statuses([ + /* existing requests */ +])Also applies to: 213-233, 274-294
test/standard-schema/standalone.test.ts (3)
160-162: Typo: "focou" vs "fouco".Use a consistent test value to avoid confusion when skimming failures.
-const invalid = await app.handle(req('/focou')) +const invalid = await app.handle(req('/fouco'))
214-218: Avoid console noise in test runs.Guarded console.log can still clutter CI output. Recommend removing or gating behind an env flag.
- .onError(({ error, code }) => { - if (code !== 'VALIDATION') console.log(error) - }) + .onError(({ code }) => { + if (code !== 'VALIDATION') {/* noop in tests */} + })
360-575: Add array/repeat-key query cases.Given new query parsing, add cases like ?ids=1&ids=2 and ?list=[1,2] to lock in behavior for arrays/repeated keys.
src/type-system/utils.ts (2)
157-161: Stale docstring reference.Doc still points to
validateFileExtension; it should referencefileType.- * to actual check the file type, use `validateFileExtension` instead + * to actually check the file type, use `fileType` instead
172-181: Consider deferring to fileType when extension is provided.Currently this only checks
value.type(lightweight). Consider offering an option to perform a deep check viafileTypeto avoid spoofed MIME types when security matters.src/error.ts (1)
446-451: Broaden model resolution for standard validators on schema-wrapped validators.If
~standardis onvalidator.schema, current check misses it.- if ('~standard' in this.validator) return this.validator + if ('~standard' in (this.validator as any)) return this.validator + // @ts-ignore + if (this.validator?.schema && '~standard' in this.validator.schema) return this.validator.schemasrc/parse-query.ts (2)
199-237: Guard charCodeAt on empty values (minor).Accessing
charCodeAt(0)on empty strings yields NaN; add a fast length check to skip JSON/CSV heuristics when empty.- if ( + if ( + finalValue.length > 1 && finalValue.charCodeAt(0) === 91 && finalValue.charCodeAt(finalValue.length - 1) === 93 ) { ... - } else if ( + } else if ( + finalValue.length > 1 && finalValue.charCodeAt(0) === 123 && finalValue.charCodeAt(finalValue.length - 1) === 125 ) { ...
1-240: Add unit tests for new query parsing variants.Please add focused tests for:
- repeated keys:
?ids=1&ids=2- CSV to array coercion:
?tags=a,b,c- JSON arrays/objects:
?list=[1,2],?filters={"a":1}src/utils.ts (2)
1-1: Remove unused import.The
isStringTextContainingNodeis imported from 'typescript' but doesn't appear to be used in the file.-import { isStringTextContainingNode } from 'typescript'
574-574: Consider more descriptive HTTP status text.While "Enhance Your Calm" is creative, consider using the standard "Enhance Your Calm (Twitter)" text to indicate this is a Twitter-specific rate limiting status code.
- 'Enhance Your Calm': 420, + 'Enhance Your Calm (Twitter)': 420,src/compose.ts (2)
660-681: Consider extracting complex query parsing logic.The query parsing logic with array and object properties detection is complex and could benefit from being extracted into a separate function for better maintainability.
function buildQueryParseProperties(schema: TSchema | undefined) { const arrayProperties: Record<string, 1> = {} const objectProperties: Record<string, 1> = {} let hasArrayProperty = false let hasObjectProperty = false if (schema) { const unwrappedSchema = unwrapImportSchema(schema) if (Kind in unwrappedSchema && unwrappedSchema.properties) { for (const [key, value] of Object.entries(unwrappedSchema.properties)) { if (hasElysiaMeta('ArrayQuery', value as TSchema)) { arrayProperties[key] = 1 hasArrayProperty = true } if (hasElysiaMeta('ObjectString', value as TSchema)) { objectProperties[key] = 1 hasObjectProperty = true } } } } return { arrayProperties, objectProperties, hasArrayProperty, hasObjectProperty } }Then use it in the main function:
const { arrayProperties, objectProperties, hasArrayProperty, hasObjectProperty } = buildQueryParseProperties(validator.query?.schema)
2020-2024: Consider caching the query parser selection.The query parser selection between
parseQueryStandardSchemaandparseQueryFromURLis done at runtime. Consider determining this at compile time if possible for better performance.You could determine the parser function name during code generation:
const queryParser = validator.query?.provider === 'standard' ? 'parseQueryStandardSchema' : 'parseQueryFromURL' // Then in the utils object: parseQueryFromURL: hasQuery ? (queryParser === 'parseQueryFromURL' ? parseQueryFromURL : parseQueryStandardSchema) : undefinedtest/macro/macro.test.ts (4)
5-5: Avoid importing from build artifacts in testsImporting
statusfrom../../dist/cjsmakes tests depend on a prior build. Prefer importing the source helper to keep tests self-contained.Apply:
-import { status } from '../../dist/cjs' +import { status } from '../../src/error'
57-58: Prefer public API over internalrouter.historyThese assertions rely on
app.router.history, which is internal and brittle. Use the publicapp.routes[0].hooksaccess (already used elsewhere in this file) for consistency and stability.Example:
-expect(app.router.history[0].hooks.parse?.length).toEqual(1) +expect(app.routes[0].hooks.parse?.length).toEqual(1)Also applies to: 73-74, 89-90, 105-106, 121-122, 137-138, 153-154, 169-170, 185-186, 201-202, 217-218, 233-234
690-691: Use an explicit condition instead of an impossible random check
Math.random() > 2is always false. Replace withfalse(or a deterministic flag) to make intent clear.- if (Math.random() > 2) return status(401) + // Intentionally unreachable branch for this test case + if (false) return status(401)
1221-1226: Fix misleading comment countThe comment says “This is 4” but lists three items and the expectation is 3. Update to prevent confusion.
- // This is 4 because + // This is 3 because // 1. lilith // 2. focou // 3. sartre from focou
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
example/a.ts(1 hunks)example/custom-response.ts(1 hunks)example/openapi.ts(1 hunks)example/websocket.ts(1 hunks)package.json(2 hunks)src/adapter/bun/compose.ts(3 hunks)src/adapter/bun/index.ts(1 hunks)src/adapter/web-standard/index.ts(0 hunks)src/compose.ts(28 hunks)src/context.ts(3 hunks)src/cookies.ts(1 hunks)src/error.ts(4 hunks)src/formats.ts(1 hunks)src/parse-query.ts(2 hunks)src/schema.ts(18 hunks)src/type-system/index.ts(5 hunks)src/type-system/types.ts(1 hunks)src/type-system/utils.ts(3 hunks)src/types.ts(27 hunks)src/utils.ts(8 hunks)src/ws/types.ts(1 hunks)test/adapter/web-standard/map-early-response.test.ts(1 hunks)test/adapter/web-standard/map-response.test.ts(1 hunks)test/aot/analysis.test.ts(1 hunks)test/aot/response.test.ts(0 hunks)test/cookie/response.test.ts(3 hunks)test/core/elysia.test.ts(1 hunks)test/core/handle-error.test.ts(7 hunks)test/core/mount.test.ts(0 hunks)test/core/native-static.test.ts(1 hunks)test/core/normalize.test.ts(2 hunks)test/extends/models.test.ts(3 hunks)test/lifecycle/derive.test.ts(2 hunks)test/lifecycle/error.test.ts(4 hunks)test/lifecycle/hook-types.test.ts(1 hunks)test/lifecycle/map-response.test.ts(1 hunks)test/lifecycle/parser.test.ts(1 hunks)test/lifecycle/resolve.test.ts(2 hunks)test/lifecycle/transform.test.ts(2 hunks)test/macro/macro.test.ts(20 hunks)test/path/group.test.ts(1 hunks)test/path/guard.test.ts(1 hunks)test/plugins/checksum.test.ts(1 hunks)test/response/redirect.test.ts(0 hunks)test/standard-schema/reference.test.ts(1 hunks)test/standard-schema/standalone.test.ts(1 hunks)test/standard-schema/validate.test.ts(1 hunks)test/sucrose/infer-body-reference.test.ts(8 hunks)test/tracer/trace.test.ts(1 hunks)test/type-system/boolean-string.test.ts(1 hunks)test/type-system/uint8array.test.ts(1 hunks)test/type-system/union-enum.test.ts(2 hunks)test/types/index.ts(32 hunks)test/types/lifecycle/soundness.ts(1 hunks)test/types/macro.ts(7 hunks)test/types/schema-standalone.ts(9 hunks)test/types/standard-schema/index.ts(1 hunks)test/validator/encode.test.ts(1 hunks)test/validator/query.test.ts(5 hunks)test/validator/response.test.ts(5 hunks)test/ws/aot.test.ts(0 hunks)
💤 Files with no reviewable changes (5)
- test/ws/aot.test.ts
- test/aot/response.test.ts
- test/core/mount.test.ts
- src/adapter/web-standard/index.ts
- test/response/redirect.test.ts
🧰 Additional context used
🧬 Code graph analysis (34)
src/adapter/bun/index.ts (1)
src/ws/index.ts (1)
createWSMessageParser(203-234)
test/type-system/uint8array.test.ts (1)
src/universal/request.ts (1)
body(81-144)
test/core/elysia.test.ts (1)
src/index.ts (2)
Elysia(184-8073)Elysia(8075-8075)
test/types/standard-schema/index.ts (1)
src/index.ts (5)
Elysia(184-8073)Elysia(8075-8075)headers(1062-1070)Cookie(8084-8084)t(8077-8077)
test/path/group.test.ts (1)
src/index.ts (3)
Elysia(184-8073)Elysia(8075-8075)t(8077-8077)
test/extends/models.test.ts (2)
src/error.ts (1)
status(72-80)src/index.ts (1)
status(8118-8118)
test/lifecycle/error.test.ts (1)
src/error.ts (1)
status(72-80)
test/lifecycle/resolve.test.ts (2)
src/error.ts (1)
status(72-80)src/index.ts (1)
status(8118-8118)
test/validator/encode.test.ts (2)
src/error.ts (1)
status(72-80)src/index.ts (1)
status(8118-8118)
test/standard-schema/validate.test.ts (2)
src/index.ts (3)
Elysia(184-8073)Elysia(8075-8075)headers(1062-1070)test/utils.ts (1)
req(1-2)
src/adapter/bun/compose.ts (1)
src/types.ts (1)
InputSchema(691-704)
test/core/normalize.test.ts (2)
src/error.ts (1)
status(72-80)src/index.ts (1)
status(8118-8118)
test/standard-schema/standalone.test.ts (2)
src/index.ts (6)
Elysia(184-8073)Elysia(8075-8075)t(8077-8077)headers(1062-1070)status(8118-8118)error(3087-3127)test/utils.ts (1)
req(1-2)
test/lifecycle/derive.test.ts (2)
src/error.ts (1)
status(72-80)src/index.ts (1)
status(8118-8118)
example/openapi.ts (1)
src/index.ts (4)
Elysia(184-8073)Elysia(8075-8075)t(8077-8077)status(8118-8118)
test/plugins/checksum.test.ts (1)
src/index.ts (2)
Elysia(184-8073)Elysia(8075-8075)
test/types/lifecycle/soundness.ts (4)
example/openapi.ts (1)
app(16-74)src/index.ts (5)
Elysia(184-8073)Elysia(8075-8075)status(8118-8118)t(8077-8077)headers(1062-1070)src/error.ts (1)
status(72-80)src/types.ts (1)
Prettify(301-303)
test/core/handle-error.test.ts (2)
src/index.ts (3)
Elysia(184-8073)Elysia(8075-8075)status(8118-8118)src/error.ts (1)
status(72-80)
test/path/guard.test.ts (2)
src/index.ts (3)
Elysia(184-8073)Elysia(8075-8075)t(8077-8077)test/utils.ts (1)
req(1-2)
test/types/index.ts (5)
src/cookies.ts (1)
Cookie(127-280)src/universal/request.ts (1)
body(81-144)src/error.ts (1)
status(72-80)src/utils.ts (1)
form(887-925)src/universal/file.ts (1)
file(95-95)
src/utils.ts (2)
src/types.ts (2)
InputSchema(691-704)RouteSchema(431-438)src/universal/file.ts (1)
length(142-146)
test/standard-schema/reference.test.ts (2)
src/index.ts (3)
Elysia(184-8073)Elysia(8075-8075)post(5726-5810)test/utils.ts (1)
req(1-2)
src/type-system/utils.ts (2)
src/types.ts (1)
MaybeArray(261-261)src/type-system/types.ts (1)
FileType(79-79)
src/context.ts (2)
src/types.ts (3)
ResolvePath(290-292)Prettify(301-303)InputSchema(691-704)src/cookies.ts (1)
Cookie(127-280)
src/schema.ts (4)
src/error.ts (1)
mapValueError(129-197)src/index.ts (7)
mapValueError(8119-8119)TSchema(8193-8193)getSchemaValidator(8096-8096)modules(8070-8072)models(461-481)t(8077-8077)error(3087-3127)src/types.ts (2)
StandardSchemaV1Like(49-61)StandardSchemaV1LikeValidate(68-72)src/utils.ts (1)
mergeDeep(52-98)
test/sucrose/infer-body-reference.test.ts (1)
src/sucrose.ts (2)
Inference(9-19)inferBodyReference(435-504)
test/validator/response.test.ts (1)
src/error.ts (1)
status(72-80)
test/types/macro.ts (2)
src/index.ts (4)
Elysia(184-8073)Elysia(8075-8075)status(8118-8118)t(8077-8077)src/error.ts (1)
status(72-80)
src/type-system/index.ts (2)
src/type-system/utils.ts (2)
compile(38-73)tryParse(21-27)src/error.ts (1)
ValidationError(249-508)
src/compose.ts (4)
src/schema.ts (2)
unwrapImportSchema(1767-1772)hasElysiaMeta(157-208)src/type-system/utils.ts (1)
validateFile(161-184)src/utils.ts (1)
mergeCookie(99-112)src/parse-query.ts (2)
parseQueryStandardSchema(121-239)parseQueryFromURL(10-115)
src/types.ts (6)
src/universal/server.ts (1)
Serve(125-125)src/error.ts (2)
status(72-80)ElysiaCustomStatusResponse(42-70)src/universal/file.ts (1)
ElysiaFile(100-147)src/utils.ts (2)
InvertedStatusMap(598-602)InvertedStatusMap(605-605)src/context.ts (1)
Context(114-228)src/schema.ts (2)
Errors(1195-1205)Errors(1315-1325)
test/macro/macro.test.ts (3)
src/index.ts (5)
Elysia(184-8073)Elysia(8075-8075)status(8118-8118)t(8077-8077)route(6542-6633)test/utils.ts (1)
req(1-2)src/error.ts (1)
status(72-80)
src/error.ts (2)
src/schema.ts (1)
ElysiaTypeCheck(37-62)src/types.ts (1)
StandardSchemaV1Like(49-61)
src/ws/types.ts (2)
src/types.ts (11)
BaseMacro(1800-1803)RouteSchema(431-438)SingletonBase(396-401)Prettify(301-303)DocumentDecoration(1518-1523)MaybeArray(261-261)TransformHandler(1258-1277)OptionalHandler(1174-1189)MapResponse(1215-1236)AfterResponseHandler(1321-1335)ErrorHandler(1341-1516)src/context.ts (1)
Context(114-228)
🪛 Biome (2.1.2)
src/compose.ts
[error] 1212-1212: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🔇 Additional comments (52)
src/formats.ts (1)
193-217: License header whitespace cleanup looks goodTrailing spaces removed; no functional changes. Thanks for the tidy-up.
test/aot/analysis.test.ts (1)
104-106: Rename to parse: 'json' looks correct.Matches the new API contract; test intent unchanged.
test/type-system/uint8array.test.ts (1)
66-69: LGTM: cleaner handler without side effectsReturning
bodydirectly keeps the test minimal and avoids noisyconsole.logoutput. No functional concerns.example/a.ts (1)
2-2: LGTM on import change.Importing from test utils here is fine given the example context.
test/type-system/union-enum.test.ts (3)
28-28: No-op formatting changeNothing to review here.
40-40: No-op boundary changeJust a block separator.
41-55: Strengthen integration assertions and add negative cases (null, missing key)
- Assert successful responses echo the input: add expect(await res1.json()).toEqual({ value: 1 }) and expect(await res2.json()).toEqual({ value: 'some' }).
- Add explicit negative tests: null and missing property should return 422 (or your canonical validation status).
- Location: test/type-system/union-enum.test.ts — the "Integrate" test.
- I could not confirm there are no other t.UnionEnum([...null...]) usages because ripgrep in the sandbox skipped files; please run locally to verify: rg -nP "t.UnionEnum\s*(\s*[[^\]]\bnull\b[^\]]]" -g '!/node_modules/' -C2
test/sucrose/infer-body-reference.test.ts (2)
75-78: LGTM: expected shape includes new flags (server/path/route/url).
170-173: LGTM: expected retains explicit false for new flags.example/custom-response.ts (1)
8-8: Label rename to 'scoped' looks consistent with new scoping semantics.No runtime impact; OK to merge.
test/plugins/checksum.test.ts (1)
307-307: Good: removed deprecatedscopedconstructor option.Matches the new constructor API and keeps the test’s intent.
test/lifecycle/hook-types.test.ts (1)
16-16: Optional chaining is a safe guard here.Keeps the assertions precise while avoiding crashes if
transformis unset. The test will still fail if the scope isn’t"scoped".Also applies to: 22-22, 28-28
test/validator/encode.test.ts (1)
37-39: Status API migration looks correctHandler now returns a typed status response; matches response map (200, 418). No further action.
test/tracer/trace.test.ts (1)
34-34: Removing constructor{ scoped: true }is fine hereThe global trace plugin still applies to routes in
b. Test intent preserved.test/extends/models.test.ts (1)
335-335: Status-based responses LGTMRoutes now return
status(400, 'ok')andstatus(400, 1)(ts-expect-error). Matches the per-status response map tests.Also applies to: 342-342
example/openapi.ts (1)
19-21: Self-reference risk infromTypes('example/openapi.ts')If
fromTypesevaluates modules, this can re-import and start the server. Use a types-only file or ensure the loader doesn’t execute code.Would you like me to split out a
example/openapi.types.tsand wirefromTypes('example/openapi.types.ts')?package.json (1)
4-4: Verify newly pinned versions exist across registriesA few versions look forward-leaning (eg, TypeScript ^5.8.3, zod ^4.1.5). Confirm availability to avoid install failures.
I can check npm for the exact versions and known advisories if you want.
Also applies to: 187-187, 191-191, 197-197, 206-209, 211-211, 218-219
src/type-system/index.ts (2)
287-298: ObjectString meta/default change looks goodSetting
elysiaMeta: 'ObjectString'and using'{}'as default is sane and simplifies defaults. No action.
340-347: Intentional change: comma parsing disabled in ArrayStringIf this deprecates comma-delimited semantics in favor of
ArrayQuery, ensure docs/release notes call it out and provide a migration note.I can draft a short migration blurb for the changelog.
test/validator/query.test.ts (3)
259-266: Optional object query test looks goodThe additionalProperties flag and optional name handling align with the new query typing. No issues.
930-944: Transform(id) change is correct and expectations updatedSwitching to Transform(t.UnionEnum(...)) with scalar decode and adjusting the assertion to object shape is consistent with the new Transform semantics.
1025-1071: “Don’t populate object query on failed validation” — solid negative-path checksBoth missing and out-of-range cases correctly assert 422. Looks good.
src/ws/types.ts (1)
124-169: WSLocalHook generic simplification looks fineConsolidating to WSLocalHook<Input, Schema, Singleton> keeps the public surface coherent with the broader macro/input refactor. No runtime impact detected.
src/types.ts (3)
48-76: StandardSchema V1 compatibility types are well-scopedIntroducing StandardSchemaV1Like/FastStandardSchemaV1Like and AnySchema/FastAnySchema cleanly widens accepted schemas while preserving fast-paths. No issues.
104-105: ElysiaConfig.serve type change is goodserve?: Partial better reflects bun Serve shape vs a bespoke PartialServe. 👍
1518-1524: DocumentDecoration as a public interfaceGood move; aligning with OpenAPIV3.OperationObject + hide flag improves DX.
test/validator/response.test.ts (2)
1-1: Import cleanup matches public APIDropping error from imports aligns with status-based API. Good.
386-407: Status helper with reason-phrase keysUsing status("I'm a teapot", ...) relies on string-key support in StatusMap/InvertedStatusMap. Since one case is ts-ignore and the other not, please verify both compile under strict TS and behave identically at runtime.
Would you like me to add assertions on response bodies (e.g., expect text 'Kirifuji Nagisa') to harden these tests?
test/lifecycle/resolve.test.ts (1)
214-216: Switch to status(418) in resolve looks goodConsistent with the new status-based flow; the assertions below cover both AOT modes.
test/path/guard.test.ts (1)
424-440: Good coverage for standaloneValidator casting via guard callbackPositive and negative paths look correct; validates schema merge and 422 on invalid.
test/path/group.test.ts (1)
251-268: Group-based standaloneValidator casting test is solidMirrors guard case; verifies param composition and 422 behavior.
test/lifecycle/derive.test.ts (1)
185-185: Derive now signals with status(418) — OKMatches the repo-wide status-based error signaling.
test/types/schema-standalone.ts (1)
872-876: Type updates to Cookie acknowledgedThe widened cookie map value type aligns with the new Context typings; specific named cookies retain concrete types. Looks good.
Also applies to: 910-913, 947-950, 1005-1009, 1047-1051, 1085-1089, 1143-1147, 1185-1189, 1227-1231
src/adapter/bun/compose.ts (1)
10-10: Type import aligns with new validator flowImporting InputSchema here matches the repo-wide shift. No issues.
test/standard-schema/reference.test.ts (3)
37-55: Zod query coercion coverage looks goodCovers valid/invalid query parsing via z.coerce.number with correct 422 expectation.
127-155: Response schema validation scenarios are solidGood use of per-status schemas and negative case (422) for mismatched payloads.
1-5: Resolved —postis exported from test/utils.tstest/utils.ts contains
export const post = ...(and test/utils.d.ts declares it); no change required.test/core/handle-error.test.ts (1)
144-153: LGTM: thrown status mapped to body and codeCovers ElysiaCustomStatusResponse propagation correctly.
test/types/lifecycle/soundness.ts (1)
231-322: Type-level response unions look consistent across scopesThe unions for Volatile/Ephemeral/Metadata responses reconcile as expected. Nice coverage.
test/cookie/response.test.ts (2)
285-293: Good: exercise raw JSON cookie parsingSwitching to raw JSON in Cookie header validates the new auto-parse path.
393-401: LGTM: no Set-Cookie when value unchangedAsserting no new Set-Cookie when duplicate value is correct.
test/standard-schema/validate.test.ts (1)
6-302: Solid coverage for standard-schema validation paths.Scenarios and expected statuses look correct and align with the new standard validator behavior.
src/type-system/utils.ts (1)
1-200: No lingering references — validateFileExtension not found; call sites use fileTypeSearch results show fileType usages at src/type-system/utils.ts:136, src/compose.ts:1424, src/compose.ts:1468. No occurrences of validateFileExtension were found.
src/parse-query.ts (1)
1-240: Verify parseQueryFromURL call sites (startIndex + array/object hints)
- Found calls: src/compose.ts:687 — parseQueryFromURL(c.url, c.qi+1, /* array/object args */) and src/compose.ts:2231 — parseQueryFromURL(c.url, c.qi+1).
- Both pass startIndex (c.qi+1). Ensure the second call intentionally omits the optional array/object hints or add the same third/fourth args when that codepath needs them.
test/types/macro.ts (1)
119-120: Fix parameter mismatch in macro resolution.The
statusparameter is being used but the resolver function appears to accept no parameters. This creates a type mismatch.Apply this diff to fix the parameter mismatch:
- resolve: ({ status }) => { + resolve: ({ status }: { status: typeof status }) => { if (Math.random() > 0.5) return status(401)Likely an incorrect or invalid review comment.
test/types/standard-schema/index.ts (1)
1-505: LGTM! Comprehensive standard-schema type testing.Excellent coverage of Elysia's standard-schema typing across multiple configuration modes including route-level inline schema, model-based schema, guard-based schema, standalone schema, and plugin-integrated standalone schemas. The tests thoroughly validate TypeScript types for request parts and responses using expect-type and ts-expect-error checks.
test/types/index.ts (2)
1416-1418: LGTM! Correct usage of form wrapper for file payloads.The change to use
form()wrapper for file payloads aligns with the broader API evolution to handle form data correctly.
1174-1176: Macro object-syntax usage is correct.
test/types/index.ts uses app.macro({ a(a: string) {} }) and .macro({ b(a: number) {} }) and includes a ts-expect-error validating typing — no change required.src/utils.ts (2)
73-81: LGTM! Array merging logic is well-implemented.The new
mergeArrayoption inmergeDeepcorrectly handles array concatenation when enabled, providing flexible merge behavior based on the use case.
1141-1154: LGTM! Clean implementation of response length calculation.The
getResponseLengthfunction correctly handles response body streaming to calculate content length for HEAD requests.src/compose.ts (2)
252-263: LGTM! Well-structured standard provider validation.The standard provider validation path correctly handles async validation, properly awaits promises, and includes comprehensive error handling with issues propagation.
1424-1425: Good fix: Consistent fileType validator usage.The replacement of
validateFileExtensionwithfileTypeprovides consistent naming across the codebase.Also applies to: 1468-1469
example/a.ts
Outdated
| ({ q }) => { | ||
| return { q: 'a' } | ||
| }, | ||
| { | ||
| response: { | ||
| 200: t.Object({ | ||
| name: t.String() | ||
| }), | ||
| 401: t.Object({ | ||
| q: t.String() | ||
| }) | ||
| } |
There was a problem hiding this comment.
Response-schema mismatch: handler returns { q } but 200-schema expects { name }.
Without setting a non-200 status, this will violate the declared response type. Fix by returning { name } (simplest), or explicitly set 401 before returning { q }.
Apply one of the following diffs:
Option A — align with 200 schema:
- ({ q }) => {
- return { q: 'a' }
+ () => {
+ return { name: 'a' }Option B — return 401 to match the 401 schema:
- ({ q }) => {
- return { q: 'a' }
+ ({ set }) => {
+ set.status = 401
+ return { q: 'a' }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ({ q }) => { | |
| return { q: 'a' } | |
| }, | |
| { | |
| response: { | |
| 200: t.Object({ | |
| name: t.String() | |
| }), | |
| 401: t.Object({ | |
| q: t.String() | |
| }) | |
| } | |
| () => { | |
| return { name: 'a' } | |
| }, | |
| { | |
| response: { | |
| 200: t.Object({ | |
| name: t.String() | |
| }), | |
| 401: t.Object({ | |
| q: t.String() | |
| }) | |
| } |
| ({ q }) => { | |
| return { q: 'a' } | |
| }, | |
| { | |
| response: { | |
| 200: t.Object({ | |
| name: t.String() | |
| }), | |
| 401: t.Object({ | |
| q: t.String() | |
| }) | |
| } | |
| ({ set }) => { | |
| set.status = 401 | |
| return { q: 'a' } | |
| }, | |
| { | |
| response: { | |
| 200: t.Object({ | |
| name: t.String() | |
| }), | |
| 401: t.Object({ | |
| q: t.String() | |
| }) | |
| } |
🤖 Prompt for AI Agents
In example/a.ts around lines 6 to 17, the handler currently returns { q } but
the 200 response schema requires { name }, causing a schema mismatch; fix by
either (Option A) change the returned object to match the 200 schema (e.g.,
return { name: q } or otherwise populate name) or (Option B) explicitly set the
response status to 401 before returning { q } so it matches the 401 schema; pick
one option and apply the corresponding change throughout the handler.
| } | ||
| } | ||
| ) | ||
| .listen(3000) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t start a server on import
Top-level listen(3000) creates side effects (eg, during tooling or type extraction). Gate it behind a main check.
- .listen(3000)
+if (import.meta.main) {
+ app.listen(3000)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .listen(3000) | |
| if (import.meta.main) { | |
| app.listen(3000) | |
| } |
🤖 Prompt for AI Agents
In example/openapi.ts around line 74, the file calls .listen(3000) at top-level
which causes side effects on import; remove the top-level listen call, export
the Express/Fastify app instance instead, and add a guarded startup block at the
bottom that only calls app.listen(3000) when the file is executed directly (use
a main-check such as if (typeof require !== 'undefined' ? require.main ===
module : import.meta?.main) { app.listen(3000) } or the project’s preferred
ESM/CJS pattern). Ensure the server port and any startup logging remain inside
that guarded block.
|
|
||
| if (validator.query) { | ||
| if (validator.query.hasDefault) | ||
| if (Kind in validator.query?.schema && validator.query.hasDefault) |
There was a problem hiding this comment.
Fix unsafe optional chaining that could throw TypeError.
The static analysis correctly identifies that optional chaining with Kind in validator.query?.schema is unsafe. If validator.query?.schema is undefined, the in operator will throw a TypeError.
Apply this diff to fix the unsafe optional chaining:
- if (Kind in validator.query?.schema && validator.query.hasDefault)
+ if (validator.query?.schema && Kind in validator.query.schema && validator.query.hasDefault)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Kind in validator.query?.schema && validator.query.hasDefault) | |
| if (validator.query?.schema && Kind in validator.query.schema && validator.query.hasDefault) |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1212-1212: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🤖 Prompt for AI Agents
In src/compose.ts around line 1212, the expression "Kind in
validator.query?.schema" is unsafe because the optional-chained operand can be
undefined and cause a TypeError; change the condition to first ensure
validator.query?.schema is defined (e.g. check "validator.query?.schema != null"
or "validator.query && validator.query.schema") and only then use the in
operator against that schema (for example: "validator.query?.schema != null &&
Kind in validator.query.schema"), preserving the existing hasDefault check and
surrounding logic.
| `let vac=validator.cookie.Check(c.body)\n` + | ||
| `if(vac instanceof Promise)vac=await vac\n` + | ||
| `if(vac.issues){` + | ||
| validation.validate('cookie', undefined, 'vac.issues') + | ||
| '}else{c.body=vac.value}\n' |
There was a problem hiding this comment.
Fix copy-paste error in cookie validation.
There's a copy-paste error where c.body is being checked/assigned instead of the cookie value in the standard provider validation for cookies.
Apply this diff to fix the variable reference:
fnLiteral +=
- `let vac=validator.cookie.Check(c.body)\n` +
+ `let vac=validator.cookie.Check(cookieValue)\n` +
`if(vac instanceof Promise)vac=await vac\n` +
`if(vac.issues){` +
validation.validate('cookie', undefined, 'vac.issues') +
- '}else{c.body=vac.value}\n'
+ '}else{cookieValue=vac.value}\n'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `let vac=validator.cookie.Check(c.body)\n` + | |
| `if(vac instanceof Promise)vac=await vac\n` + | |
| `if(vac.issues){` + | |
| validation.validate('cookie', undefined, 'vac.issues') + | |
| '}else{c.body=vac.value}\n' | |
| `let vac=validator.cookie.Check(cookieValue)\n` + | |
| `if(vac instanceof Promise)vac=await vac\n` + | |
| `if(vac.issues){` + | |
| validation.validate('cookie', undefined, 'vac.issues') + | |
| '}else{cookieValue=vac.value}\n' |
🤖 Prompt for AI Agents
In src/compose.ts around lines 1498-1502, the cookie validation block mistakenly
references c.body (copy-paste error); change the validator call and assignment
to use the cookie property instead (e.g., validator.cookie.Check(c.cookie) and
on success set c.cookie = vac.value) and keep the existing
validation.validate('cookie', undefined, 'vac.issues') path unchanged.
| if (isWebstandard) | ||
| findDynamicRoute += | ||
| `if(r.method==='HEAD'){` + | ||
| `const route=router.find('GET',p)\n` + | ||
| 'if(route){' + | ||
| `c.params=route.params\n` + | ||
| `const _res=route.store.handler?route.store.handler(c):route.store.compile()(c)\n` + | ||
| `if(_res)` + | ||
| 'return getResponseLength(_res).then((length)=>{' + | ||
| `_res.headers.set('content-length', length)\n` + | ||
| `return new Response(null,{status:_res.status,statusText:_res.statusText,headers:_res.headers})\n` + | ||
| '})' + | ||
| '}' + | ||
| '}' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify HEAD response implementation for web standards.
The HEAD response implementation looks correct, but ensure that getResponseLength properly handles all response types including streaming responses.
🏁 Script executed:
#!/bin/bash
# Check getResponseLength implementation and its usage
ast-grep --pattern 'function getResponseLength($_) {
$$$
}'
# Check for any special handling of streaming responses
rg -n "ReadableStream|stream" src/utils.tsLength of output: 715
Avoid consuming/locking streaming bodies for HEAD; only set Content-Length when it's known.
getResponseLength (src/utils.ts:1141–1154) currently reads the entire body and returns 0 if response.bodyUsed or !response.body — this will hang on never-ending streams (SSE/ReadableStream) and can incorrectly set Content-Length to 0.
- src/utils.ts:1141–1154 — Update getResponseLength to:
- return Number(response.headers.get('content-length')) if present;
- if response.bodyUsed and no content-length header, return undefined (don’t return 0);
- when falling back to reading the body, enforce a max-byte limit and/or timeout and bail out (return undefined) for non-terminating streams.
- src/compose.ts:2194–2207 — Only set 'content-length' on the HEAD response when getResponseLength returns a finite number; otherwise return the HEAD response without a content-length header.
[src/utils.ts:1141-1154] [src/compose.ts:2194-2207]
| // it('Convert', () => { | ||
| // expect(Value.Convert(t.BooleanString(), 'true')).toBe(true) | ||
| // expect(Value.Convert(t.BooleanString(), 'false')).toBe(false) | ||
| // }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t comment out tests; use it.skip with a reason or remove if redundant.
Commenting tests hides regressions and loses intent. Either:
- Keep the test but skip it with context (e.g., pending upstream TypeBox behavior), or
- Drop it if
Decodealready covers the same coercion semantics.
Apply one of these:
Option A — keep intent, skip with reason:
- // it('Convert', () => {
- // expect(Value.Convert(t.BooleanString(), 'true')).toBe(true)
- // expect(Value.Convert(t.BooleanString(), 'false')).toBe(false)
- // })
+ it.skip('Convert (pending upstream behavior clarification)', () => {
+ expect(Value.Convert(t.BooleanString(), 'true')).toBe(true)
+ expect(Value.Convert(t.BooleanString(), 'false')).toBe(false)
+ })Option B — remove if redundant with Decode:
- // it('Convert', () => {
- // expect(Value.Convert(t.BooleanString(), 'true')).toBe(true)
- // expect(Value.Convert(t.BooleanString(), 'false')).toBe(false)
- // })To check for other occurrences of commented-out tests and Value.Convert usage:
🏁 Script executed:
#!/bin/bash
# Find commented-out tests
rg -nP -C2 '^\s*//\s*(it|test)\s*\('
# Find Value.Convert usages
rg -nP 'Value\.Convert\s*\(' -C2Length of output: 15970
Don't comment out tests — use it.skip with a reason or remove if redundant.
Convert the commented block in test/type-system/boolean-string.test.ts (lines 66–69) to a skipped test with context or delete it; the repo contains many other commented tests that need the same treatment.
Option A — keep intent, skip with reason:
- // it('Convert', () => {
- // expect(Value.Convert(t.BooleanString(), 'true')).toBe(true)
- // expect(Value.Convert(t.BooleanString(), 'false')).toBe(false)
- // })
+ it.skip('Convert (pending upstream behavior clarification)', () => {
+ expect(Value.Convert(t.BooleanString(), 'true')).toBe(true)
+ expect(Value.Convert(t.BooleanString(), 'false')).toBe(false)
+ })Option B — remove if redundant with Decode:
- // it('Convert', () => {
- // expect(Value.Convert(t.BooleanString(), 'true')).toBe(true)
- // expect(Value.Convert(t.BooleanString(), 'false')).toBe(false)
- // })Other occurrences found (apply same treatment): test/units/class-to-object.test.ts, test/type-system/uint8array.test.ts, test/ws/message.test.ts, test/validator/query.test.ts, test/sucrose/infer-body-reference.test.ts, test/sucrose/retrieve-root-parameters.test.ts, test/tracer/aot.test.ts, test/lifecycle/response.test.ts, test/lifecycle/derive.test.ts, test/extends/error.test.ts, test/path/path.test.ts, test/cookie/signature.test.ts, test/cookie/response.test.ts, test/core/normalize.test.ts, test/core/elysia.test.ts, etc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // it('Convert', () => { | |
| // expect(Value.Convert(t.BooleanString(), 'true')).toBe(true) | |
| // expect(Value.Convert(t.BooleanString(), 'false')).toBe(false) | |
| // }) | |
| it.skip('Convert (pending upstream behavior clarification)', () => { | |
| expect(Value.Convert(t.BooleanString(), 'true')).toBe(true) | |
| expect(Value.Convert(t.BooleanString(), 'false')).toBe(false) | |
| }) |
🤖 Prompt for AI Agents
In test/type-system/boolean-string.test.ts around lines 66–69, there is a
commented-out test block; replace it with a skipped test (or remove if
redundant) rather than leaving it commented. Restore the test body and
assertions into an it.skip call with a brief reason (e.g., "Convert test
deferred — duplicates Decode" or "flaky/needs update"), or delete the block if
you confirm Decode covers the behavior; apply the same treatment (convert
commented tests to it.skip with reasons or delete) to the other listed files to
remove commented-out tests repository-wide.
| SSEPayload, | ||
| status, | ||
| form |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the removal of SSEPayload from imports.
The SSEPayload type is imported but commented out. Since this appears to be part of the API change from error to status, ensure this removal is intentional and that no dependent code relies on this export.
🏁 Script executed:
#!/bin/bash
# Check if SSEPayload is still used anywhere in the codebase
rg -n "SSEPayload" --type ts --type jsLength of output: 610
Restore SSEPayload import or update all usages
SSEPayload is still referenced in the codebase; removal is unsafe. Affected locations:
- src/utils.ts (lines: 23, 1061, 1063, 1084, 1092, 1095, 1097, 1099, 1107, 1110)
- src/index.ts:8175
- src/types.ts:2419
- test/types/index.ts:8
Either reintroduce/export/import SSEPayload or refactor every usage to the new type/API before removing it.
🤖 Prompt for AI Agents
In test/types/index.ts around lines 8 to 10, the SSEPayload export was removed
but the type is still referenced across the codebase; restore the SSEPayload
export (re-add it to the export list and import it from its source module, e.g.,
src/types.ts) or, if the type was renamed, update all usages to the new name
across the listed files (src/utils.ts, src/index.ts, src/types.ts and test
files) so references compile; ensure the export/import path is correct and run
type checks to verify no remaining references to the old name.
| resolve: async ({ status }) => { | ||
| if (Math.random() > 0.5) return status(401) |
There was a problem hiding this comment.
Fix async function parameter mismatch.
Similar to the previous issue, the async resolver has a parameter mismatch.
Apply this diff:
- resolve: async ({ status }) => {
+ resolve: async ({ status }: { status: typeof status }) => {
if (Math.random() > 0.5) return status(401)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resolve: async ({ status }) => { | |
| if (Math.random() > 0.5) return status(401) | |
| resolve: async ({ status }: { status: typeof status }) => { | |
| if (Math.random() > 0.5) return status(401) |
🤖 Prompt for AI Agents
In test/types/macro.ts around lines 149-150, the async resolver currently
destructures status from the first parameter but the framework passes it as the
second argument; update the function signature to accept the expected parameters
(e.g., async (_req, { status }) => { ... }) so status is taken from the second
parameter and retains the existing logic returning status(401) when
Math.random() > 0.5.
| if (Math.random() > 0.5) return status(401) | ||
|
|
There was a problem hiding this comment.
Fix status parameter type in resolve array.
The status parameter in the resolve array also has the same type mismatch issue.
Apply this diff:
resolve: [
- ({ status }) => {
+ ({ status }: { status: typeof status }) => {
if (Math.random() > 0.5) return status(401)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ({ status }) => { | |
| if (Math.random() > 0.5) return status(401) | |
| ({ status }: { status: typeof status }) => { | |
| if (Math.random() > 0.5) return status(401) |
🤖 Prompt for AI Agents
In test/types/macro.ts around lines 240-241, the resolve array callback
currently types the status parameter incorrectly; change the callback signature
so status is typed as a number (e.g., status: number) or the correct function
signature that accepts a numeric status code, then return status(401) as before
so the type matches the actual use of a numeric HTTP status code.
| it("don't populate array query on failed validation", async () => { | ||
| const app = new Elysia().get('/', ({ query }) => query, { | ||
| query: t.Object({ | ||
| party: t.Array( | ||
| t.Object({ | ||
| name: t.String() | ||
| }) | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| const filter = JSON.stringify([ | ||
| { | ||
| name: 'lilith' | ||
| }, | ||
| { | ||
| name: 'fouco' | ||
| } | ||
| ]) | ||
|
|
||
| const valid = await app.handle( | ||
| new Request(`http://localhost:3000/?party=${filter}`) | ||
| ) | ||
| const invalid1 = await app.handle(new Request(`http://localhost:3000`)) | ||
| const invalid2 = await app.handle( | ||
| new Request( | ||
| `http://localhost:3000?filter=[]` | ||
| ) | ||
| ) | ||
|
|
||
| expect(valid.status).toBe(200) | ||
| expect(invalid1.status).toBe(422) | ||
| expect(invalid2.status).toBe(422) | ||
| }) |
There was a problem hiding this comment.
Bug: wrong query key used in invalid array test (likely masking intent)
invalid2 requests ?filter=[] but schema expects party: t.Array(...). This tests a missing required key (same as invalid1), not a wrong-shaped array. Use ?party=[] to truly exercise array-shape failure.
Apply this diff:
- const invalid2 = await app.handle(
- new Request(
- `http://localhost:3000?filter=[]`
- )
- )
+ const invalid2 = await app.handle(
+ new Request(
+ `http://localhost:3000?party=[]`
+ )
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("don't populate array query on failed validation", async () => { | |
| const app = new Elysia().get('/', ({ query }) => query, { | |
| query: t.Object({ | |
| party: t.Array( | |
| t.Object({ | |
| name: t.String() | |
| }) | |
| ) | |
| }) | |
| }) | |
| const filter = JSON.stringify([ | |
| { | |
| name: 'lilith' | |
| }, | |
| { | |
| name: 'fouco' | |
| } | |
| ]) | |
| const valid = await app.handle( | |
| new Request(`http://localhost:3000/?party=${filter}`) | |
| ) | |
| const invalid1 = await app.handle(new Request(`http://localhost:3000`)) | |
| const invalid2 = await app.handle( | |
| new Request( | |
| `http://localhost:3000?filter=[]` | |
| ) | |
| ) | |
| expect(valid.status).toBe(200) | |
| expect(invalid1.status).toBe(422) | |
| expect(invalid2.status).toBe(422) | |
| }) | |
| it("don't populate array query on failed validation", async () => { | |
| const app = new Elysia().get('/', ({ query }) => query, { | |
| query: t.Object({ | |
| party: t.Array( | |
| t.Object({ | |
| name: t.String() | |
| }) | |
| ) | |
| }) | |
| }) | |
| const filter = JSON.stringify([ | |
| { | |
| name: 'lilith' | |
| }, | |
| { | |
| name: 'fouco' | |
| } | |
| ]) | |
| const valid = await app.handle( | |
| new Request(`http://localhost:3000/?party=${filter}`) | |
| ) | |
| const invalid1 = await app.handle(new Request(`http://localhost:3000`)) | |
| const invalid2 = await app.handle( | |
| new Request( | |
| `http://localhost:3000?party=[]` | |
| ) | |
| ) | |
| expect(valid.status).toBe(200) | |
| expect(invalid1.status).toBe(422) | |
| expect(invalid2.status).toBe(422) | |
| }) |
🤖 Prompt for AI Agents
test/validator/query.test.ts around lines 1073 to 1106: the third request uses
the wrong query key (?filter=[]) while the schema expects party, so it
duplicates the missing-key test instead of exercising an invalid array shape;
update the invalid2 request to use ?party=[] (i.e.
Request('http://localhost:3000?party=[]')) so the test truly validates
array-shape failure and asserts a 422 response.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/types.ts (3)
2193-2200: Fix discriminant in ModelValidator.safeParse.Error branch must be
success: falsefor proper narrowing.- | { - success: true - data: null - error: string - errors: ModelValidatorError[] - } + | { + success: false + data: null + error: string + errors: ModelValidatorError[] + }
2392-2393: HTTP header key typo: 'alt-svg' → 'alt-svc'.This breaks correct typing for Alt-Svc.
- 'alt-svg'?: string + 'alt-svc'?: string
227-233: Doc fixes: sanitize spelling, arrayBuffer token, minor grammar.
- "sanitlize" → "sanitize"
'arraybuffer'→'arrayBuffer'to match the union- "it have performance impact" → "it has a performance impact"
- * This option only works when `sanitlize` is `exactMirror` + * This option only works when `sanitize` is `exactMirror`- * - 'urlencoded' / 'application/x-www-form-urlencoded: parse body as urlencoded + * - 'urlencoded' / 'application/x-www-form-urlencoded': parse body as urlencoded - * - 'arraybuffer': parse body as readable stream + * - 'arrayBuffer': parse body as readable stream- * - 'typebox': Since this uses dynamic Value.Clean, it have performance impact + * - 'typebox': Since this uses dynamic Value.Clean, it has a performance impactAlso applies to: 1572-1583, 201-204
♻️ Duplicate comments (1)
src/types.ts (1)
1149-1153: Remove stray resolve.q from InlineHandler context (public Context pollution).The extra
& { resolve: { q: Prettify<Route> } }leaks a bogusresolve.qinto user-facingContext, conflicting withMacroContext['resolve']. Remove it.- Singleton & { resolve: MacroContext['resolve'] } & { - resolve: { - q: Prettify<Route> - } - } + Singleton & { resolve: MacroContext['resolve'] }
🧹 Nitpick comments (4)
src/types.ts (1)
2260-2264: Duplicate MIME in SetContentType.
'video/x-msvideo'appears twice; keep one.| 'video/x-msvideo' | 'video/quicktime' | 'video/x-ms-wmv' - | 'video/x-msvideo' | 'video/x-flv'example/a.ts (3)
6-7: Make the example deterministic (avoid Math.random in auth).Flip to a simple header-based check so examples/docs don’t behave nondeterministically.
- resolve: ({ status }) => - Math.random() > 0.5 ? { role: 'user' } : status(401, 'not authorized') + resolve: ({ headers, status }) => + headers.authorization?.startsWith('Bearer ') + ? { role: 'user' } + : status(401, 'not authorized')
9-12: Return a structured body and declare response schemas.Improves DX and prevents schema drift as the example evolves.
- .post('/', ({ role }) => role, { - auth: true, - beforeHandle: ({ role }) => {} - }) + .post('/', ({ role }) => ({ role }), { + auth: true, + response: { + 200: t.Object({ role: t.String() }), + 401: t.Literal('not authorized') + } + })
11-11: Drop the no-op beforeHandle.It adds noise; either remove it or enforce auth there (return 401) if you want an explicit guard.
- beforeHandle: ({ role }) => {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
example/a.ts(1 hunks)src/types.ts(27 hunks)test/types/lifecycle/soundness.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/types/lifecycle/soundness.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/types.ts (6)
src/universal/server.ts (1)
Serve(125-125)src/error.ts (2)
status(72-80)ElysiaCustomStatusResponse(42-70)src/universal/file.ts (1)
ElysiaFile(100-147)src/utils.ts (2)
InvertedStatusMap(598-602)InvertedStatusMap(605-605)src/context.ts (1)
Context(114-228)src/schema.ts (2)
Errors(1195-1205)Errors(1315-1325)
example/a.ts (2)
src/index.ts (4)
Elysia(184-8080)Elysia(8082-8082)t(8084-8084)status(8125-8125)src/error.ts (1)
status(72-80)
🔇 Additional comments (2)
example/a.ts (2)
3-8: Auth macro wiring looks good.Macro name matches the route option key, header validator will be applied as a standalone validator, and resolve will merge payloads into context.
6-7: Keep ctx.status — no change required.Context exposes
status(src/error.ts exportsstatus; src/context.ts includesstatus: typeof status), so destructuring ({ status }) in example/a.ts is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/lifecycle/response.test.ts (1)
161-169: Wrong hook: this test targets onAfterResponse but uses onAfterHandleThe case won’t detect regressions in onAfterResponse’s array support.
Apply this diff:
- .onAfterHandle([ + .onAfterResponse([src/compose.ts (1)
1925-1929: Bug: awaiting the wrong variable in error mapResponse path.This awaits
erinstead ofmep.- mep=e.mapResponse[${i}](c)\n - if(mep instanceof Promise)er=await er\n + mep=e.mapResponse[${i}](c)\n + if(mep instanceof Promise)mep=await mep\n if(mep!==undefined)er=mep\n
♻️ Duplicate comments (6)
src/types.ts (1)
1147-1154: Remove stray resolve.q pollution from InlineHandler context.This introduces an unintended
qfield underresolveon public Context.- Singleton & { resolve: MacroContext['resolve'] } & { - resolve: { - q: Prettify<Route> - } - } + Singleton & { resolve: MacroContext['resolve'] }src/compose.ts (4)
1211-1213: Fix unsafe optional chaining with theinoperator.
Kind in validator.query?.schemacan throw whenschemais undefined.- if (Kind in validator.query?.schema && validator.query.hasDefault) + if (validator.query?.schema && Kind in validator.query.schema && validator.query.hasDefault)
2284-2290: HEAD handling (static routes): mirror the Content-Length guard.- 'return getResponseLength(_res).then((length)=>{' + - `_res.headers.set('content-length', length)\n` + + 'return getResponseLength(_res).then((length)=>{' + + `if (Number.isFinite(Number(length))) _res.headers.set('content-length', String(length))\n` + `return new Response(null,{status:_res.status,statusText:_res.statusText,headers:_res.headers})\n` + '})\n'
2195-2207: HEAD handling: only set Content-Length when known.Avoid setting an incorrect length for streaming/unknown bodies.
- 'return getResponseLength(_res).then((length)=>{' + - `_res.headers.set('content-length', length)\n` + + 'return getResponseLength(_res).then((length)=>{' + + `if (Number.isFinite(Number(length))) _res.headers.set('content-length', String(length))\n` + `return new Response(null,{status:_res.status,statusText:_res.statusText,headers:_res.headers})\n` + '})' +
1496-1504: Standard cookie validation mutates c.body and drops validated values; write back to c.cookie.Use
cookieValuefor validation and assign results back to the cookie jar.- let vac=validator.cookie.Check(c.body) + let vac=validator.cookie.Check(cookieValue) if(vac instanceof Promise)vac=await vac if(vac.issues){ ${'/* keep existing ValidationError construction */'} - ${''} - }else{c.body=vac.value} + }else{ + for (const [k, v] of Object.entries(vac.value as Record<string, unknown>)) { + if (k in c.cookie) c.cookie[k].value = v as any + } + }src/context.ts (1)
140-145: Headers fallback checks the wrong resolve key.Use
resolve.headers(notresolve.query) in the default branch.- ? {} extends NonNullable<Singleton['resolve']['query']> + ? {} extends NonNullable<Singleton['resolve']['headers']> ? Record<string, string | undefined> - : Singleton['resolve']['headers'] + : Singleton['resolve']['headers']
🧹 Nitpick comments (2)
test/lifecycle/map-response.test.ts (1)
333-333: Nit: fix test name typo ("mapResonse" → "mapResponse")Keeps test output tidy.
- it('handle set in mapResonse', async () => { + it('handle set in mapResponse', async () => {example/a.ts (1)
6-13: Example logic never reaches the handler; make success path possible.As written, requests either 400 in resolve or 401 in beforeHandle; the POST handler is unreachable.
- resolve: ({ status }) => - Math.random() > 0.5 ? { role: 'user' } : status(400) + resolve: ({ status }) => + Math.random() > 0.5 ? { role: 'admin' } : status(400)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
example/a.ts(1 hunks)package.json(2 hunks)src/compose.ts(42 hunks)src/context.ts(3 hunks)src/types.ts(30 hunks)test/lifecycle/after-handle.test.ts(1 hunks)test/lifecycle/map-response.test.ts(3 hunks)test/lifecycle/response.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
test/lifecycle/response.test.ts (2)
src/index.ts (3)
Elysia(184-8080)Elysia(8082-8082)t(8084-8084)test/utils.ts (1)
req(1-2)
src/types.ts (5)
src/error.ts (2)
status(72-80)ElysiaCustomStatusResponse(42-70)src/universal/file.ts (1)
ElysiaFile(100-147)src/utils.ts (2)
InvertedStatusMap(598-602)InvertedStatusMap(605-605)src/context.ts (1)
Context(113-227)src/schema.ts (2)
Errors(1195-1205)Errors(1315-1325)
src/compose.ts (4)
src/schema.ts (2)
unwrapImportSchema(1767-1772)hasElysiaMeta(157-208)src/type-system/utils.ts (1)
validateFile(161-184)src/utils.ts (1)
mergeCookie(99-112)src/parse-query.ts (2)
parseQueryStandardSchema(121-239)parseQueryFromURL(10-115)
test/lifecycle/after-handle.test.ts (2)
test/utils.ts (1)
req(1-2)src/index.ts (2)
Elysia(184-8080)Elysia(8082-8082)
example/a.ts (2)
src/index.ts (4)
Elysia(184-8080)Elysia(8082-8082)t(8084-8084)status(8125-8125)src/error.ts (1)
status(72-80)
src/context.ts (2)
src/types.ts (3)
ResolvePath(290-292)Prettify(301-303)InputSchema(691-704)src/cookies.ts (1)
Cookie(127-280)
test/lifecycle/map-response.test.ts (2)
src/index.ts (2)
Elysia(184-8080)Elysia(8082-8082)test/utils.ts (1)
req(1-2)
🪛 Biome (2.1.2)
src/compose.ts
[error] 1212-1212: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🔇 Additional comments (11)
test/lifecycle/response.test.ts (2)
63-87: LGTM: afterResponse inherits from plugin and exposes final value via responseCovers the global plugin scope and confirms the final value type is preserved.
89-113: LGTM: variant using responseValue is correctGood parallel coverage ensuring responseValue reflects the raw handler return.
test/lifecycle/map-response.test.ts (5)
106-117: LGTM: inherit mapping via responseValueValidates mapping from the raw value; assertions are correct.
139-157: LGTM: inherit set + responseValueConfirms headers from set are preserved alongside mapping from responseValue.
274-293: LGTM: error-path mapping using responseValueGood coverage ensuring custom error values can be converted to Response.
300-307: LGTM: mapResponse builds a Response explicitlyEnsures the after-handle path yields a concrete Response.
309-314: LGTM: after-handle + responseValue interopConfirms responseValue is available and usable post afterHandle.
test/lifecycle/after-handle.test.ts (2)
91-101: LGTM: accepts response while no-op mapResponse presentConfirms afterHandle can pass through the existing response.
102-110: LGTM: accepts responseValueValidates exposure of responseValue in afterHandle without altering outcome.
package.json (2)
4-4: Pre-release version — already published under a non-latest dist-tagnpm dist-tags show experimental: "1.4.0-exp.18" and latest: "1.3.21", so the prerelease did not bump the latest tag.
191-209: Re-check dev dependency availability — rerun using exact versions (strip range operators).The prior run returned WARN for @elysiajs/openapi, arktype, valibot, zod, and typescript while confirming exact-mirror@0.2.2; those WARNs are likely from searching for the literal caret (^) in the JSON. Re-run a check that queries each exact version and confirm resolution against your registry mirror:
#!/bin/bash set -euo pipefail pkgs=( "@elysiajs/openapi@1.3.11" "arktype@2.1.22" "valibot@1.1.0" "zod@4.1.5" "typescript@5.8.3" "exact-mirror@0.2.2" ) for p in "${pkgs[@]}"; do echo "Checking $p" if npm view "$p" version >/dev/null 2>&1; then npm view "$p" version else echo "MISSING: $p" fi doneRun this from CI or with the same registry mirror (NPM_CONFIG_REGISTRY) and report any MISSING lines.
| // ! Get latest app.config.cookie | ||
| const cookieValidator = getCookieValidator({ | ||
| // @ts-expect-error private property | ||
| modules: app.definitions.typebox, | ||
| validator: validator.cookie as any, | ||
| defaultConfig: app.config.cookie, | ||
| dynamic: !!app.config.aot, | ||
| config: validator.cookie?.config ?? {}, | ||
| normalize: app.config.normalize, | ||
| // @ts-expect-error | ||
| models: app.definitions.type | ||
| })! | ||
| validator.cookie.config = mergeCookie( | ||
| validator.cookie.config, | ||
| validator.cookie?.config ?? {} | ||
| ) | ||
|
|
There was a problem hiding this comment.
Cookie config merge uses the same source twice; merge with app config.
This currently merges validator.cookie.config with itself, ignoring app.config.cookie.
- validator.cookie.config = mergeCookie(
- validator.cookie.config,
- validator.cookie?.config ?? {}
- )
+ validator.cookie.config = mergeCookie(
+ validator.cookie.config,
+ (app.config.cookie as any) ?? {}
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ! Get latest app.config.cookie | |
| const cookieValidator = getCookieValidator({ | |
| // @ts-expect-error private property | |
| modules: app.definitions.typebox, | |
| validator: validator.cookie as any, | |
| defaultConfig: app.config.cookie, | |
| dynamic: !!app.config.aot, | |
| config: validator.cookie?.config ?? {}, | |
| normalize: app.config.normalize, | |
| // @ts-expect-error | |
| models: app.definitions.type | |
| })! | |
| validator.cookie.config = mergeCookie( | |
| validator.cookie.config, | |
| validator.cookie?.config ?? {} | |
| ) | |
| // ! Get latest app.config.cookie | |
| validator.cookie.config = mergeCookie( | |
| validator.cookie.config, | |
| (app.config.cookie as any) ?? {} | |
| ) |
🤖 Prompt for AI Agents
In src/compose.ts around lines 1482-1487, the merge call is incorrectly using
validator.cookie.config twice; replace the second argument so it merges the
app's cookie config instead of itself. Change the call to
mergeCookie(validator.cookie.config, app.config?.cookie ?? {}) (or the correct
app config variable in scope) so the validator's cookie config is merged with
app.config.cookie, making the access null-safe with a fallback empty object.
|
lgtm 👍 
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
package.json (3)
187-187: Avoid dual declaringexact-mirroras both dependency and peerHaving it in both can cause duplicate installs or resolution conflicts. Prefer peerDependency (consumer-provided) with a devDependency for local builds, or keep as a regular dependency and drop the peer. Pick one strategy.
Option A — peer only (plus devDependency for local use):
"dependencies": { "cookie": "^1.0.2", - "exact-mirror": "0.2.2", "fast-decode-uri-component": "^1.0.1" }, "devDependencies": { + "exact-mirror": "0.2.2",If you keep it as a peer, also add:
+ "peerDependenciesMeta": { + "exact-mirror": { "optional": false } + },
197-197: Multiple validator libs — ensure tree-shaking and example coverageAdding
arktype,valibot,zodis fine; confirm examples/tests cover each provider so unused ones don’t inflate consumers’ bundles. Consider marking them as peer+optional if intended to be consumer-chosen.
218-219: Constrain optional deps to major to avoid surprise breaksOpenAPI types and TypeBox should be upper-bounded to their major.
- "@sinclair/typebox": ">= 0.34.0 < 1", - "openapi-types": ">= 12.0.0" + "@sinclair/typebox": ">= 0.34.0 < 0.35", + "openapi-types": ">= 12.0.0 < 13"example/a.ts (3)
6-8: Prefer integer constraint foridUse
t.Integer()(or Number with min/step) to prevent decimals.- params: t.Object({ - id: t.Number() - }) + params: t.Object({ + id: t.Integer() + })
11-15: Optional: declare response schema for example completenessHelps type inference and OpenAPI generation.
- app.get('/:name', ({ params }) => params, { - params: t.Object({ - name: t.String() - }) - }) + app.get('/:name', ({ params }) => params, { + params: t.Object({ name: t.String() }), + response: { + 200: t.Object({ id: t.Integer(), name: t.String() }) + } + })
3-16: Optional: start or export the app for the dev script
bun run --watch example/a.tswon’t serve without.listenor an export.Examples:
- start:
new Elysia()...listen(3000)- or export:
export const app = new Elysia()...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
example/a.ts(1 hunks)package.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example/a.ts (1)
src/index.ts (3)
Elysia(184-8080)Elysia(8082-8082)t(8084-8084)
🔇 Additional comments (5)
package.json (3)
4-4: Version bump to 1.4.0 — OKLooks consistent with the scope of changes. Ensure release notes and migration guide reflect API changes.
206-208: Deps published — confirm CI/compatibilityVerified: typescript@5.8.3, valibot@1.1.0, zod@4.1.5 are published on npm. File: package.json (lines 206–208). Run your CI/local builds across the target Node/TS matrix (tsconfig, lockfile) and fix any compilation or transitive-dependency failures.
191-191: ```shell
#!/bin/bash
set -euo pipefailP="@elysiajs/openapi@^1.3.11"
N="${P%@}"
V="${P#@}"
version="${V#^}"echo "Checking $P -> name='$N' version='$version'"
Build candidate encodings (try raw and percent-encoded forms)
candidates=("$N")
if command -v python3 >/dev/null 2>&1; then
candidates+=("$(python3 - <<'PY'
import urllib.parse,sys
print(urllib.parse.quote(sys.argv[1], safe=''))
PY
"$N")")
candidates+=("$(python3 - <<'PY'
import sys
print(sys.argv[1].replace('/','%2F').replace('@','%40'))
PY
"$N")")
else
candidates+=("${N//@/%40}")
candidates+=("${N////%2F}")
fiseen=()
for name in "${candidates[@]}"; do
url="https://registry.npmjs.org/$name"avoid duplicate attempts
if printf '%s\n' "${seen[@]}" | grep -qx "$url"; then
continue
fi
seen+=("$url")
echo "Trying: $url"
body=$(curl -s -L "$url" || true)If JSON, inspect metadata for the requested version
if echo "$body" | jq -e . >/dev/null 2>&1; then
echo "✔ JSON received from $url"
echo "$body" | jq -r --arg v "$version" '
if .versions[$v] then
.versions[$v] | {
version: $v,
has_types_field: (has("types") or has("typings")),
types_field_value: (.types // .typings // null),
dist_tarball: (.dist.tarball // null),
files: (.files // null)
}
else
"version-missing"
end
'
exit 0
else
echo "✖ Response not JSON (len=$(echo -n "$body" | wc -c))"
fi
doneecho "Failed to retrieve JSON metadata for package/version"
exit 2</blockquote></details> <details> <summary>example/a.ts (2)</summary><blockquote> `1-1`: **Import from package — good** Switching to public import path is correct for examples. --- `3-16`: **Validate that group-level and route-level `params` are merged at runtime** From `src/index.ts`, the group path applies `standaloneValidator` from the group schema; depending on `mergeHook`, the route’s own `params` may be overwritten. If so, `name` wouldn’t be validated. Please confirm merge behavior; if not merged, either: - put both params on the route, or - explicitly merge schemas. Example (route-level only): ```diff - { - params: t.Object({ id: t.Number() }) - }, - (app) => - app.get('/:name', ({ params }) => params, { - params: t.Object({ name: t.String() }) - }) + (app) => + app.get('/id/:id/:name', ({ params }) => params, { + params: t.Object({ id: t.Number(), name: t.String() }) + })
| }, | ||
| "peerDependencies": { | ||
| "@sinclair/typebox": ">= 0.34.0", | ||
| "@sinclair/typebox": ">= 0.34.0 < 1", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
TypeBox range too broad for 0.x (minors can be breaking)
Widening to < 1 risks breakage when 0.35+ lands. Safer to cap within 0.34.x.
- "@sinclair/typebox": ">= 0.34.0 < 1",
+ "@sinclair/typebox": ">= 0.34.0 < 0.35",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@sinclair/typebox": ">= 0.34.0 < 1", | |
| "@sinclair/typebox": ">= 0.34.0 < 0.35", |
🤖 Prompt for AI Agents
In package.json at line 211, the TypeBox semver range ">= 0.34.0 < 1" is too
broad for 0.x releases; change it to restrict updates within the 0.34.x line
(for example ">=0.34.0 <0.35.0") so minor releases in 0.35+ cannot be pulled in
and risk breaking changes.
1 participant
Release name: Supersymmetry
Codename: Weirs (Tone Sphere)
Summary by CodeRabbit