1.4.5 patch #1403
1.4.5 patch #1403
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughUpdated docs and bumped version to 1.4.5. Core changes: cookie validation now mutates and propagates decoded cookie values; schema import/ref resolution refined and JSON-string defaults removed; deepClone util added; new IntersectIfObjectSchema types and expanded route/group typings; multiple tests and examples updated to model-driven t.* types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Server as Elysia
participant Composer as src/compose.ts
participant Validator as StandardValidator
participant CookieBag as c.cookie
Client->>Server: HTTP request (Cookie header)
Server->>Composer: extract cookieValue
Composer->>Validator: validate(cookieValue)
alt validation succeeds
Validator-->>Composer: vac.value
Composer->>Composer: cookieValue = vac.value
Composer->>CookieBag: for each key set c.cookie[key].value = cookieValue[key]
alt validator.cookie.hasTransform
Composer->>Validator: Decode(cookieValue)
Validator-->>Composer: decoded values
Composer->>CookieBag: assign decoded c.cookie[*].value
end
Composer-->>Server: proceed with updated cookie values
Server-->>Client: handler response
else validation fails
Validator-->>Server: error (422)
Server-->>Client: error response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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: 2
🧹 Nitpick comments (5)
CHANGELOG.md (1)
1-4: Polish the 1.4.5 entry (date + wording).To match prior entries and improve clarity:
-# 1.4.5 -Bug fix: -- standard schema incorrectly validate cookie and coercion +# 1.4.5 - 14 Sep 2025 +Fix: +- standard schema: incorrect cookie validation/coercionpackage.json (1)
4-4: Version/changelog mismatch (beta vs stable).CHANGELOG says 1.4.5 (stable) but package.json is 1.4.5-beta.0. Confirm intent and align both artifacts.
Would you like me to update CHANGELOG to “1.4.5-beta.0 - 14 Sep 2025”, or bump package.json to 1.4.5?
src/compose.ts (1)
1499-1505: Likely merge target mistake when refreshing validator cookie config.This merges the config with itself; intention appears to be merging in
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 + )example/a.ts (1)
4-14: Tidy example: correct typeof, drop stray log, and use absolute route.-const app = new Elysia().get( - 'test', - ({ cookie: { test } }) => { - console.log(test.value) - - return typeof test - }, +const app = new Elysia().get( + '/test', + ({ cookie: { test } }) => typeof test.value, { cookie: z.object({ test: z.coerce.number() }) } )src/cookies.ts (1)
263-265: Expose a stable public API — don't write to a private setter from generated codecompose.ts emits
c.cookie[k].innerValue=...(src/compose.ts:1524) while src/cookies.ts defines a private setter at lines ~263–265; expose a public helper and update the generator to call it.- private set innerValue(value: T) { - this.value = value - } + /** Assign value post-validation without mutating other options */ + public assignValidated(value: T) { + this.value = value + }Replace generator/compose usages of
.innerValuewith.assignValidated(...).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)package.json(1 hunks)src/compose.ts(2 hunks)src/cookies.ts(1 hunks)test/standard-schema/validate.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/standard-schema/validate.test.ts (1)
src/cookies.ts (2)
value(154-156)value(158-160)
example/a.ts (1)
src/cookies.ts (2)
value(154-156)value(158-160)
🔇 Additional comments (4)
test/standard-schema/validate.test.ts (1)
303-323: Good addition: exercises cookie coercion path.Covers zod
z.coerce.number()from Cookie header to typed value. Looks solid.src/compose.ts (3)
1507-1510: Collecting cookieValue is correct.Building a plain object snapshot from
c.cookiebefore validation is the right move.
1515-1521: Standard-provider validation flow LGTM.Validates with standard schema and uses the validated value.
1526-1530: Bug: cookie branch checks body schema flag.This should reference
validator.cookie, notvalidator.body.-} else if (validator.body?.schema?.noValidate !== true) { +} else if (validator.cookie?.schema?.noValidate !== true) {Likely an incorrect or invalid review comment.
| if (validator.cookie.hasTransform) | ||
| fnLiteral += coerceTransformDecodeError( | ||
| `for(const [key,value] of Object.entries(validator.cookie.Decode(cookieValue))){` + | ||
| `c.cookie[key].value=value` + | ||
| `}`, | ||
| 'cookie' | ||
| ) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure propagation for non-standard providers without transform; unify assignment.
After a successful non-standard Check(cookieValue), propagate values into c.cookie even when no transform is present. Also prefer the public setter:
- if (validator.cookie.hasTransform)
+ // Always reflect validated values
+ for (const k of Object.keys(cookieValue))
+ c.cookie[k].value = cookieValue[k]
+
+ if (validator.cookie.hasTransform)
${coerceTransformDecodeError(
`for(const [key,value] of Object.entries(validator.cookie.Decode(cookieValue))){` +
- `c.cookie[key].value=value` +
+ `c.cookie[key].value=value` +
`}`,
'cookie'
)}If you prefer minimal churn, I can submit a follow-up PR with tests covering the non-standard provider path.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for occurrences of 'validator.cookie' and 'cookieValue'..."
rg -n --hidden "validator\.cookie" -S || true
rg -n --hidden "\bcookieValue\b" -S || true
echo
echo "Show src/compose.ts around the reported lines (1400-1560):"
if [ -f src/compose.ts ]; then
sed -n '1400,1560p' src/compose.ts || true
else
echo "src/compose.ts not found"
fiLength of output: 6862
Propagate validated cookie values for non-standard providers and unify assignment
When validator.cookie.Check succeeds for non-standard providers we must reflect the validated cookieValue into c.cookie for all keys (use the public .value setter), then let Decode override it when hasTransform is true — current code only updates on hasTransform and uses inconsistent properties.
- if (validator.cookie.hasTransform)
+ // Always reflect validated values
+ for (const k of Object.keys(cookieValue))
+ c.cookie[k].value = cookieValue[k]
+
+ if (validator.cookie.hasTransform)
${coerceTransformDecodeError(
`for(const [key,value] of Object.entries(validator.cookie.Decode(cookieValue))){` +
- `c.cookie[key].value=value` +
+ `c.cookie[key].value=value` +
`}`,
'cookie'
)}src/compose.ts (around lines ~1523-1536).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/compose.ts around lines 1531 to 1538, when validator.cookie.Check
succeeds for non-standard providers we must first propagate the validated
cookieValue into c.cookie for every key using the public .value setter (e.g.,
for(const [k,v] of Object.entries(cookieValue)){ c.cookie[k].value = v }), then
if validator.cookie.hasTransform is true call Decode and let its results
override those same keys (again assigning via the .value setter); replace the
current conditional that only updates on hasTransform and ensure all assignments
use the unified .value property and the coerceTransformDecodeError-wrapped
Decode invocation so transforms still run and override validated values.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/schema.ts (1)
1108-1115: Bug: hasRef getter calls hasTransform.
get hasRef()returnshasTransform(schema)instead ofhasRef(schema). This misreports metadata and can mislead downstream logic.- return (this['~hasRef'] = hasTransform(schema)) + return (this['~hasRef'] = hasRef(schema))
🧹 Nitpick comments (7)
src/utils.ts (1)
1166-1171: Redundant typeof check.
typeof source === 'function'is implied bytypeof source !== 'object'; you can drop it for clarity.example/a.ts (2)
5-11: Example won’t coerce string numerics with t.Number(); use t.Numeric() (or send numbers).As written, sending strings ('1', '2') will fail validation. Either switch to t.Numeric() or send numeric JSON.
const app = new Elysia() .model({ - q: t.Number(), + q: t.Numeric(), number: t.Object({ - value: t.Number(), + value: t.Numeric(), number: t.Ref('q') }) })
14-20: Optional: assert output instead of console.log for a runnable sample.Consider converting this to an assertion or comment expected output to avoid noisy logs in examples.
test/validator/query.test.ts (1)
1098-1099: Use the correct query key to keep the test meaningful.The schema expects
party, but the invalid request usesfilter, which weakens the test’s intent. Use?party=[]to assert failed population for the targeted field.- const invalid2 = await app.handle( - new Request(`http://localhost:3000?filter=[]`) - ) + const invalid2 = await app.handle( + new Request(`http://localhost:3000?party=[]`) + )test/extends/models.test.ts (1)
228-251: Don’t silently disable the “coerce with reference model” test.If behavior changed, replace with an updated assertion or add a TODO explaining why it’s skipped. Silent commenting hides regressions.
src/schema.ts (2)
535-555: ObjectString/ArrayString default injection removed: verify transform still applied.Transform is now attached only when
v.defaultequals{}or[]. Ensuret.ObjectString/t.ArrayStringprovide those defaults by design; otherwise, transforms may be skipped. Consider an explicit flag instead of default-string checks.
822-844: Potential gap: coercion skipped for Import with nested refs.In the Import branch, if the referenced def still has refs, coercion isn’t applied. This appears intentional (tests now use
t.Numeric()), but it’s a behavior change. Please confirm and document: “coercion is not applied inside referenced models; use t.Numeric()/t.BooleanString in models.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)src/schema.ts(3 hunks)src/utils.ts(1 hunks)test/extends/models.test.ts(4 hunks)test/validator/query.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (3)
src/schema.ts (2)
src/index.ts (4)
TSchema(8201-8201)model(7552-7637)t(8085-8085)modules(8078-8080)src/utils.ts (1)
randomId(927-930)
test/extends/models.test.ts (1)
src/index.ts (3)
Elysia(184-8081)Elysia(8083-8083)t(8085-8085)
example/a.ts (3)
src/index.ts (4)
Elysia(184-8081)Elysia(8083-8083)t(8085-8085)post(5733-5818)src/type-system/index.ts (1)
t(657-657)test/utils.ts (1)
post(46-67)
🔇 Additional comments (3)
test/validator/query.test.ts (1)
838-840: Good: reference model now uses Numeric.Switching to
t.Numeric()for both the model and the ref removes reliance on implicit coercion.test/extends/models.test.ts (2)
256-256: Good: defaulted numeric reference uses t.Numeric().This aligns defaults with new coercion semantics.
284-286: Good: cookie model token switched to Numeric.Prevents relying on implicit numeric coercion in cookies.
|
|
||
| export function deepClone<T>(source: T, weak = new WeakMap<object, any>()): T { | ||
| if ( | ||
| source === null || | ||
| typeof source !== 'object' || | ||
| typeof source === 'function' | ||
| ) | ||
| return source | ||
|
|
||
| // Circular‑reference guard | ||
| if (weak.has(source as object)) return weak.get(source as object) | ||
|
|
||
| if (Array.isArray(source)) { | ||
| const copy: any[] = new Array(source.length) | ||
| weak.set(source, copy) | ||
|
|
||
| for (let i = 0; i < source.length; i++) | ||
| copy[i] = deepClone(source[i], weak) | ||
|
|
||
| return copy as any | ||
| } | ||
|
|
||
| if (typeof source === 'object') { | ||
| const keys = Object.keys(source).concat( | ||
| Object.getOwnPropertySymbols(source) as any[] | ||
| ) | ||
|
|
||
| const cloned: Partial<T> = {} | ||
|
|
||
| for (const key of keys) | ||
| cloned[key as keyof T] = deepClone((source as any)[key], weak) | ||
|
|
||
| return cloned as T | ||
| } | ||
|
|
||
| return source | ||
| } |
There was a problem hiding this comment.
Fix circular reference for plain objects (WeakMap set happens too late).
Objects aren’t added to the WeakMap before descending, so self/recursive structures will loop indefinitely. Set the placeholder before recursing (arrays already do this).
export function deepClone<T>(source: T, weak = new WeakMap<object, any>()): T {
@@
if (Array.isArray(source)) {
const copy: any[] = new Array(source.length)
weak.set(source, copy)
@@
return copy as any
}
if (typeof source === 'object') {
- const keys = Object.keys(source).concat(
- Object.getOwnPropertySymbols(source) as any[]
- )
-
- const cloned: Partial<T> = {}
-
- for (const key of keys)
- cloned[key as keyof T] = deepClone((source as any)[key], weak)
-
- return cloned as T
+ const keys = Object.keys(source).concat(
+ Object.getOwnPropertySymbols(source) as any[]
+ )
+ const cloned: Partial<T> = {}
+ weak.set(source as object, cloned)
+ for (const key of keys) {
+ (cloned as any)[key] = deepClone((source as any)[key as any], weak)
+ }
+ return cloned as T
}
return source
}📝 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.
| export function deepClone<T>(source: T, weak = new WeakMap<object, any>()): T { | |
| if ( | |
| source === null || | |
| typeof source !== 'object' || | |
| typeof source === 'function' | |
| ) | |
| return source | |
| // Circular‑reference guard | |
| if (weak.has(source as object)) return weak.get(source as object) | |
| if (Array.isArray(source)) { | |
| const copy: any[] = new Array(source.length) | |
| weak.set(source, copy) | |
| for (let i = 0; i < source.length; i++) | |
| copy[i] = deepClone(source[i], weak) | |
| return copy as any | |
| } | |
| if (typeof source === 'object') { | |
| const keys = Object.keys(source).concat( | |
| Object.getOwnPropertySymbols(source) as any[] | |
| ) | |
| const cloned: Partial<T> = {} | |
| for (const key of keys) | |
| cloned[key as keyof T] = deepClone((source as any)[key], weak) | |
| return cloned as T | |
| } | |
| return source | |
| } | |
| export function deepClone<T>(source: T, weak = new WeakMap<object, any>()): T { | |
| if ( | |
| source === null || | |
| typeof source !== 'object' || | |
| typeof source === 'function' | |
| ) | |
| return source | |
| // Circular‑reference guard | |
| if (weak.has(source as object)) return weak.get(source as object) | |
| if (Array.isArray(source)) { | |
| const copy: any[] = new Array(source.length) | |
| weak.set(source, copy) | |
| for (let i = 0; i < source.length; i++) | |
| copy[i] = deepClone(source[i], weak) | |
| return copy as any | |
| } | |
| if (typeof source === 'object') { | |
| const keys = Object.keys(source).concat( | |
| Object.getOwnPropertySymbols(source) as any[] | |
| ) | |
| const cloned: Partial<T> = {} | |
| weak.set(source as object, cloned) | |
| for (const key of keys) { | |
| (cloned as any)[key] = deepClone((source as any)[key as any], weak) | |
| } | |
| return cloned as T | |
| } | |
| return source | |
| } |
🤖 Prompt for AI Agents
In src/utils.ts around lines 1164 to 1200, the deepClone implementation adds
plain objects to the WeakMap only after recursing, causing infinite loops on
self-referential structures; fix it by creating the cloned placeholder object
(e.g., const cloned: Partial<T> = {}) and immediately calling weak.set(source as
object, cloned) before iterating over keys and calling deepClone for each
property, mirroring the array branch’s behavior so circular references are
handled correctly.
| it("coerce model when there's no reference", async () => { | ||
| const app = new Elysia() | ||
| .model({ | ||
| idParam: t.Object({ | ||
| id: t.Union([ | ||
| t.String({ format: 'uuid' }), | ||
| t.Number({ | ||
| minimum: 1, | ||
| maximum: Number.MAX_SAFE_INTEGER | ||
| }) | ||
| ]) | ||
| }), | ||
| response200: t.Object({ | ||
| message: t.String(), | ||
| content: t.Array( | ||
| t.Object({ | ||
| id: t.Union([t.String(), t.Number()]) | ||
| }) | ||
| ) | ||
| }) | ||
| }) | ||
| .get( | ||
| '/:id', | ||
| ({ params: { id } }) => ({ | ||
| message: 'ok', | ||
| content: [{ id }] | ||
| }), | ||
| { | ||
| params: 'idParam', | ||
| response: 'response200' | ||
| } | ||
| ) | ||
| .listen(3000) | ||
|
|
||
| const value = await app | ||
| .handle(new Request('http://localhost/3')) | ||
| .then((x) => x.json()) | ||
|
|
||
| expect(value).toEqual({ message: 'ok', content: [{ id: 3 }] }) | ||
| }) |
There was a problem hiding this comment.
Avoid binding a real port in tests (.listen(3000)).
Use app.handle(...) without .listen() to keep tests hermetic and prevent flaky port conflicts.
)
- .listen(3000)
const value = await app
.handle(new Request('http://localhost/3'))
.then((x) => x.json())📝 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("coerce model when there's no reference", async () => { | |
| const app = new Elysia() | |
| .model({ | |
| idParam: t.Object({ | |
| id: t.Union([ | |
| t.String({ format: 'uuid' }), | |
| t.Number({ | |
| minimum: 1, | |
| maximum: Number.MAX_SAFE_INTEGER | |
| }) | |
| ]) | |
| }), | |
| response200: t.Object({ | |
| message: t.String(), | |
| content: t.Array( | |
| t.Object({ | |
| id: t.Union([t.String(), t.Number()]) | |
| }) | |
| ) | |
| }) | |
| }) | |
| .get( | |
| '/:id', | |
| ({ params: { id } }) => ({ | |
| message: 'ok', | |
| content: [{ id }] | |
| }), | |
| { | |
| params: 'idParam', | |
| response: 'response200' | |
| } | |
| ) | |
| .listen(3000) | |
| const value = await app | |
| .handle(new Request('http://localhost/3')) | |
| .then((x) => x.json()) | |
| expect(value).toEqual({ message: 'ok', content: [{ id: 3 }] }) | |
| }) | |
| it("coerce model when there's no reference", async () => { | |
| const app = new Elysia() | |
| .model({ | |
| idParam: t.Object({ | |
| id: t.Union([ | |
| t.String({ format: 'uuid' }), | |
| t.Number({ | |
| minimum: 1, | |
| maximum: Number.MAX_SAFE_INTEGER | |
| }) | |
| ]) | |
| }), | |
| response200: t.Object({ | |
| message: t.String(), | |
| content: t.Array( | |
| t.Object({ | |
| id: t.Union([t.String(), t.Number()]) | |
| }) | |
| ) | |
| }) | |
| }) | |
| .get( | |
| '/:id', | |
| ({ params: { id } }) => ({ | |
| message: 'ok', | |
| content: [{ id }] | |
| }), | |
| { | |
| params: 'idParam', | |
| response: 'response200' | |
| } | |
| ) | |
| const value = await app | |
| .handle(new Request('http://localhost/3')) | |
| .then((x) => x.json()) | |
| expect(value).toEqual({ message: 'ok', content: [{ id: 3 }] }) | |
| }) |
🤖 Prompt for AI Agents
In test/extends/models.test.ts around lines 359 to 398, the test binds a real
port by calling .listen(3000) which can cause flaky tests and port conflicts;
remove the .listen(3000) call and use the Elysia app instance directly with
app.handle(new Request(...)) (or await app.ready() if required by framework) so
the test remains hermetic and does not open a network port.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/utils.ts (1)
1188-1223: Critical:deepCloneloops on circular plain objects; set WeakMap entry before descending.Arrays are guarded, but plain objects aren’t added to
weakuntil after recursion, so self-references recurse forever. Register the placeholder first.Apply:
if (typeof source === 'object') { const keys = Object.keys(source).concat( Object.getOwnPropertySymbols(source) as any[] ) - const cloned: Partial<T> = {} - - for (const key of keys) - cloned[key as keyof T] = deepClone((source as any)[key], weak) + const cloned: Partial<T> = {} + weak.set(source as object, cloned) + for (const key of keys) { + (cloned as any)[key] = deepClone((source as any)[key as any], weak) + } return cloned as T }Minor nit: the guard redundantly checks
typeof source === 'function'which is already covered bytypeof source !== 'object'.- if ( - source === null || - typeof source !== 'object' || - typeof source === 'function' - ) + if (source === null || typeof source !== 'object') return source
🧹 Nitpick comments (4)
src/utils.ts (1)
310-329: SimplifystandaloneSchemamerge; current nested ternary is hard to read and error-prone.Equivalent, clearer logic:
- // @ts-ignore - standaloneSchema: - // @ts-ignore - a.standaloneSchema || b.standaloneSchema - ? // @ts-ignore - a.standaloneSchema && !b.standaloneSchema - ? // @ts-ignore - a.standaloneSchema - : // @ts-ignore - b.standaloneSchema && !a.standaloneSchema - ? b.standaloneSchema - : [ - // @ts-ignore - ...(a.standaloneSchema ?? []), - ...(b.standaloneSchema ?? []) - ] - : undefined + standaloneSchema: + (a.standaloneSchema?.length || b.standaloneSchema?.length) + ? ([...(a.standaloneSchema ?? []), ...(b.standaloneSchema ?? [])] as any) + : undefinedsrc/index.ts (1)
4642-4644: Duplicate standalone schema detection logic.The
hasStandaloneSchemacheck is duplicated from the group method. Consider extracting this into a helper function to maintain DRY principles.Consider extracting the schema detection logic:
private hasStandaloneSchema(schema: AnyLocalHook): boolean { const { body, headers, query, params, cookie, response } = schema; return !!(body || headers || query || params || cookie || response); }Then use it in both methods:
- const hasStandaloneSchema = - body || headers || query || params || cookie || response + const hasStandaloneSchema = this.hasStandaloneSchema(hook)example/a.ts (2)
1-3: Clean up unused import.The
zodimport on line 2 is no longer used after migrating to thettype system.import { Elysia, t } from '../src' -import z from 'zod' import { req } from '../test/utils'
40-44: Consider uncommenting the assertion for clarity.The commented assertion would make the expected behavior clearer for documentation purposes.
console.log(value) -// expect(value).toEqual({ -// name: 'lilith', -// playing: true, -// limit: 10 -// }) +// Expected output: +// { +// name: 'lilith', +// playing: true, +// limit: 10 +// }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
example/a.ts(1 hunks)src/index.ts(4 hunks)src/utils.ts(2 hunks)test/path/group.test.ts(1 hunks)test/path/guard.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
test/path/group.test.ts (2)
src/index.ts (4)
Elysia(184-8098)Elysia(8100-8100)t(8102-8102)error(3087-3127)test/utils.ts (1)
req(1-2)
src/index.ts (2)
src/utils.ts (1)
mergeHook(220-334)src/types.ts (1)
AnyLocalHook(1571-1571)
test/path/guard.test.ts (2)
src/index.ts (4)
Elysia(184-8098)Elysia(8100-8100)t(8102-8102)error(3087-3127)test/utils.ts (1)
req(1-2)
example/a.ts (3)
src/index.ts (3)
Elysia(184-8098)Elysia(8100-8100)t(8102-8102)src/type-system/index.ts (1)
t(657-657)test/utils.ts (1)
req(1-2)
🔇 Additional comments (10)
src/utils.ts (2)
265-274: Confirm precedence changes for local validators and detail.
- Local validators now prefer
b.* ?? a.*(body/headers/params/query/cookie) whiletypeprefersa.type || b.type. Please confirm this asymmetry is intentional.detailis merged asmergeDeep(b.detail ?? {}, a.detail ?? {}), meaninga.detailoverridesb.detail. Verify this matches your merge semantics.Also applies to: 281-287
289-309: LGTM: Hook arrays merged without side effects and deduped via checksum seed.Using
mergeObjectArrayfor parse/transform/afterHandle/mapResponse/afterResponse/trace/error looks correct and avoids in-place mutation.test/path/guard.test.ts (1)
442-479: Test demonstrates correct nested guard schema merging behavior.The test validates that:
- Multiple nested guards properly compose their query schemas (name, limit, playing)
- All schema constraints are enforced (literal 'lilith', numeric limit, boolean playing)
- Missing required fields correctly return 422 status
This aligns with the framework's guard composition semantics.
test/path/group.test.ts (2)
270-290: LGTM! Test validates basic nested group functionality.The test confirms that nested groups without schemas properly pass through query parameters to the handler.
292-332: Comprehensive test coverage for nested guard groups with schemas.The test properly validates:
- Schema inheritance across nested groups (outer → inner → route-level)
- Correct merging of query validators at each level
- Appropriate error handling when required fields are missing
The test structure mirrors the guard test, ensuring consistent behavior across both APIs.
src/index.ts (4)
4040-4042: LGTM! Proper conditional logic for standalone schema detection.The
hasStandaloneSchemaflag correctly identifies when any schema component is present, which determines whether to append a new standalone validator entry.
4631-4640: Consistent variable naming improves code clarity.The renaming from
hookstolocalHookand extraction ofguardHookmakes the code more readable and clearly distinguishes between the route's local hooks and the guard-level hooks being applied.
4662-4674: Consistent standalone validator handling across guard and group.The implementation correctly mirrors the group method's approach, ensuring consistent behavior when composing standalone validators in nested guard contexts.
4060-4073: Approve: standalone validator array construction confirmed. Found insertStandaloneValidator (src/utils.ts) and matching merges/pushes in src/index.ts and adapter/bun/compose.ts; the conditional preserves existing validators when no schema and appends the new validator object when schema components exist.example/a.ts (1)
5-33: Example effectively demonstrates nested guard schema composition.The code shows how multiple guard levels can progressively add schema requirements:
- Outer guard requires
name: 'lilith'- Middle guard adds
limit: Number- Route-level schema adds
playing: BooleanThis mirrors the test cases and provides a clear usage example.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/index.ts (1)
4650-4694: Guard merge should also retain guardHook.standaloneValidator.As with group(), guard merge overwrites any guard-specified standaloneValidator. Merge both sources first.
Apply this diff:
- const hasStandaloneSchema = + const hasStandaloneSchema = body || headers || query || params || cookie || response + const existingStandalone = [ + ...(guardHook.standaloneValidator ?? []), + ...(localHook.standaloneValidator ?? []) + ] ... - mergeHook(guardHook as AnyLocalHook, { + mergeHook(guardHook as AnyLocalHook, { ...((localHook || {}) as AnyLocalHook), ... - standaloneValidator: !hasStandaloneSchema - ? localHook.standaloneValidator - : [ - ...(localHook.standaloneValidator ?? []), - { - body, - headers, - query, - params, - cookie, - response - } - ] + standaloneValidator: hasStandaloneSchema + ? [ + ...existingStandalone, + { body, headers, query, params, cookie, response } + ] + : existingStandalone
♻️ Duplicate comments (1)
src/utils.ts (1)
254-256: Good fix: avoid calling Object.values on possibly undefined.Swapping the null-guards before
Object.values(b)prevents a runtime TypeError.
🧹 Nitpick comments (8)
CHANGELOG.md (1)
1-12: Polish wording and confirm release date.
- Suggest clearer phrasing:
- “standard schema incorrectly validate cookie and coercion” → “fix incorrect cookie validation/coercion in Standard Schema”
- “overwrite primitive value if collide” → “overwrite primitive values on conflict”
- Date says “15 Sep 2025” while PR opened on 14 Sep 2025. Please confirm final release date before merge.
-# 1.4.5 - 15 Sep 2025 +# 1.4.5 - 15 Sep 2025 Improvement: - soundness for guard, group -- overwrite primitive value if collide (intersectIfObject) +- overwrite primitive values on conflict (intersectIfObject) Bug fix: -- standard schema incorrectly validate cookie and coercion +- fix incorrect cookie validation/coercion in Standard Schema - merge nested guard, group standalone schema properly Breaking Change: - no longer coerce type for .model with `t.Ref` by defaultsrc/types.ts (1)
708-716: Refine IntersectIfObject to match existing object checks.Current condition uses
Record<any, any>and may behave unexpectedly for non-plain objects. Reuse the existingIsBothObject<A,B>helper to keep semantics consistent with other merge utilities.-export type IntersectIfObject<A, B> = - A extends Record<any, any> - ? B extends Record<any, any> - ? A & B - : A - : B extends Record<any, any> - ? B - : A +export type IntersectIfObject<A, B> = + IsBothObject<A, B> extends true + ? A & B + : A extends Record<any, any> + ? A + : B extends Record<any, any> + ? B + : Asrc/utils.ts (3)
284-287: Consider precedence fordetailmerge (minor).Elsewhere, local
bfields take precedence (b.body ?? a.body), butdetailcurrently doesmergeDeep(b.detail, a.detail)which letsaoverridebwhen keys collide. If local should win, flip the order:- detail: mergeDeep( - b.detail ?? {}, - a.detail ?? {} - ), + detail: mergeDeep( + a.detail ?? {}, + b.detail ?? {} + ),
310-329: Simplify standaloneSchema merge (readability).The nested conditional is correct but hard to scan. Consider normalizing to arrays then concatenating:
- standaloneSchema: - a.standaloneSchema || b.standaloneSchema - ? a.standaloneSchema && !b.standaloneSchema - ? a.standaloneSchema - : b.standaloneSchema && !a.standaloneSchema - ? b.standaloneSchema - : [ ...(a.standaloneSchema ?? []), ...(b.standaloneSchema ?? []) ] - : undefined + standaloneSchema: (() => { + const A = a.standaloneSchema ?? [] + const B = b.standaloneSchema ?? [] + return A.length || B.length ? ([...A, ...B] as any) : undefined + })()
1188-1224: deepClone: solid baseline with circular-reference guard; note non-plain objects.
- Good: arrays pre‑registered in WeakMap; objects registered before descent.
- Caveat: special objects (Date/RegExp/Map/Set/TypedArray/ArrayBuffer) and property descriptors aren’t preserved. If deepClone will only handle plain JSON-like data, add a brief comment; otherwise, add targeted handlers.
Example handler stubs (optional):
+ // Handle common built-ins + if (source instanceof Date) return new Date(source.getTime()) as any + if (source instanceof RegExp) return new RegExp(source) as any + if (source instanceof Map) { + const m = new Map() + weak.set(source as object, m) + for (const [k, v] of source) m.set(deepClone(k as any, weak), deepClone(v as any, weak)) + return m as any + } + if (source instanceof Set) { + const s = new Set() + weak.set(source as object, s) + for (const v of source) s.add(deepClone(v as any, weak)) + return s as any + }src/index.ts (3)
4048-4050: Preserve any existing standaloneValidator from schemaOrRun when merging.Current merge path discards schemaOrRun.standaloneValidator in favor of localHook’s. Merge both before appending the synthesized validator.
Apply this diff:
- const hasStandaloneSchema = + const hasStandaloneSchema = body || headers || query || params || cookie || response + const existingStandalone = [ + ...(hook.standaloneValidator ?? []), + ...(localHook.standaloneValidator ?? []) + ] ... - standaloneValidator: !hasStandaloneSchema - ? localHook.standaloneValidator - : [ - ...(localHook.standaloneValidator ?? - []), - { - body, - headers, - query, - params, - cookie, - response - } - ] + standaloneValidator: hasStandaloneSchema + ? [ + ...existingStandalone, + { + body, + headers, + query, + params, + cookie, + response + } + ] + : existingStandaloneAlso applies to: 4069-4081
6721-6739: ws MacroContext missing Definitions param.MacroToContext here omits Definitions['typebox'] (present elsewhere), which may degrade ref resolution in macro-driven schemas for ws.
Apply this diff:
- const MacroContext extends MacroToContext< - Metadata['macroFn'], - Omit<Input, NonResolvableMacroKey> - > + const MacroContext extends MacroToContext< + Metadata['macroFn'], + Omit<Input, NonResolvableMacroKey>, + Definitions['typebox'] + >
5670-5685: Optional: consider reusing a shared Schema alias to reduce duplication.All route builders repeat the same IntersectIfObjectSchema generic. A top-level type alias (e.g., RouteBuilderSchema<BasePath, Path, Input, Definitions, Volatile, Ephemeral, Metadata>) would improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)package.json(1 hunks)src/index.ts(20 hunks)src/types.ts(2 hunks)src/utils.ts(3 hunks)test/path/guard.test.ts(1 hunks)test/types/index.ts(3 hunks)test/types/lifecycle/soundness.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- test/path/guard.test.ts
🧰 Additional context used
🧬 Code graph analysis (3)
test/types/lifecycle/soundness.ts (2)
src/index.ts (4)
Elysia(186-8139)Elysia(8141-8141)t(8143-8143)status(8184-8184)src/error.ts (1)
status(72-80)
src/types.ts (1)
src/index.ts (1)
RouteSchema(8208-8208)
src/index.ts (2)
src/types.ts (9)
MergeSchema(726-765)MacroToContext(1008-1077)NonResolvableMacroKey(1651-1653)LocalHook(1661-1722)Prettify(301-303)AnyLocalHook(1589-1589)IntersectIfObjectSchema(717-724)UnwrapRoute(525-564)JoinPath(2460-2463)src/utils.ts (1)
mergeHook(220-334)
🔇 Additional comments (9)
src/types.ts (2)
528-529: Variance tweak looks consistent.Using “in out” on Path aligns with existing generics in this file. No issues.
717-724: Schema-level intersection helper looks good.Field-wise application of IntersectIfObject is clear and matches intent. Ensure it’s exported from the public surface where needed (e.g., index.ts) to avoid type import duplication.
test/types/index.ts (2)
867-887: LGTM: merged body shape expectation now includes username.Matches guard+inner body intersection.
1053-1081: LGTM: prefer local params schema.Switching to
t.Number()and assertingid: numberis consistent with local override.test/types/lifecycle/soundness.ts (2)
1887-1899: LGTM: standalone schema on GET coverage.Validates that standalone body schema is respected even on GET.
1901-1942: LGTM: merged guard schemas + local route schema.Type assertion for
{ playing, name, limit }confirms the new intersection utility works as intended across nesting.src/index.ts (3)
3908-3921: Schema typing in group(): good direction.Using merged route + global schemas here makes sense. No issues spotted.
4433-4444: Guard() schema typing: good.Extending Schema with merged volatile/ephemeral/metadata + standalone intersections reads correctly.
5670-5685: LGTM: IntersectIfObjectSchema across route builders.Intersecting per-route with standalone schemas only when both sides are objects tightens typings without breaking existing merges. Looks good across get/post/put/patch/delete/options/all/head/connect/route.
Also applies to: 5775-5790, 5881-5895, 5985-5999, 6089-6103, 6193-6207, 6297-6311, 6401-6415, 6505-6519, 6610-6624
| MaybeValueOrVoidFunction, | ||
| IntersectIfObject, | ||
| IntersectIfObjectSchema | ||
| } from './types' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Re-export new public types or drop unused import.
You import IntersectIfObject and IntersectIfObjectSchema for typing, but they aren’t re-exported in this file’s public surface. If the intent is to expose them (as per PR summary), add a re-export; otherwise, remove IntersectIfObject to avoid an unused type import.
Apply this diff (safe to place near other type re-exports at the bottom):
+export type { IntersectIfObject, IntersectIfObjectSchema } from './types'📝 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.
| MaybeValueOrVoidFunction, | |
| IntersectIfObject, | |
| IntersectIfObjectSchema | |
| } from './types' | |
| MaybeValueOrVoidFunction, | |
| IntersectIfObject, | |
| IntersectIfObjectSchema | |
| } from './types' | |
| export type { IntersectIfObject, IntersectIfObjectSchema } from './types' |
🤖 Prompt for AI Agents
In src/index.ts around lines 165 to 168, the types IntersectIfObject and
IntersectIfObjectSchema are imported but not re-exported; if the intent (per PR
summary) is to expose them, add them to the file’s public type re-exports near
the other type exports at the bottom (e.g., include IntersectIfObject and
IntersectIfObjectSchema in the export list); otherwise, remove these two imports
to eliminate the unused type import.
| derive: Singleton['derive'] | ||
| resolve: Prettify< | ||
| Singleton['resolve'] & | ||
| Ephemeral['resolve'] & | ||
| Volatile['resolve'] | ||
| // @ts-ignore | ||
| MacroContext['resolve'] | ||
| > | ||
| }, | ||
| Definitions, | ||
| { | ||
| schema: Schema | ||
| standaloneSchema: Metadata['standaloneSchema'] | ||
| standaloneSchema: Metadata['standaloneSchema'] & | ||
| Schema & | ||
| MacroContext | ||
| macro: Metadata['macro'] | ||
| macroFn: Metadata['macroFn'] | ||
| parser: Metadata['parser'] | ||
| response: Metadata['response'] | ||
| response: Metadata['response'] & | ||
| // @ts-ignore | ||
| MacroContext['response'] | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use SimplifyToSchema for standaloneSchema to avoid leaking non-schema keys.
StandaloneSchema currently intersects MacroContext directly, which can pull in non-schema keys (eg resolve). Prefer narrowing to schema-like keys.
Apply this diff:
- standaloneSchema: Metadata['standaloneSchema'] &
- Schema &
- MacroContext
+ standaloneSchema: Metadata['standaloneSchema'] &
+ Schema &
+ SimplifyToSchema<MacroContext>📝 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.
| derive: Singleton['derive'] | |
| resolve: Prettify< | |
| Singleton['resolve'] & | |
| Ephemeral['resolve'] & | |
| Volatile['resolve'] | |
| // @ts-ignore | |
| MacroContext['resolve'] | |
| > | |
| }, | |
| Definitions, | |
| { | |
| schema: Schema | |
| standaloneSchema: Metadata['standaloneSchema'] | |
| standaloneSchema: Metadata['standaloneSchema'] & | |
| Schema & | |
| MacroContext | |
| macro: Metadata['macro'] | |
| macroFn: Metadata['macroFn'] | |
| parser: Metadata['parser'] | |
| response: Metadata['response'] | |
| response: Metadata['response'] & | |
| // @ts-ignore | |
| MacroContext['response'] | |
| }, | |
| derive: Singleton['derive'] | |
| resolve: Prettify< | |
| Singleton['resolve'] & | |
| // @ts-ignore | |
| MacroContext['resolve'] | |
| > | |
| }, | |
| Definitions, | |
| { | |
| schema: Schema | |
| standaloneSchema: Metadata['standaloneSchema'] & | |
| Schema & | |
| SimplifyToSchema<MacroContext> | |
| macro: Metadata['macro'] | |
| macroFn: Metadata['macroFn'] | |
| parser: Metadata['parser'] | |
| response: Metadata['response'] & | |
| // @ts-ignore | |
| MacroContext['response'] | |
| }, |
🤖 Prompt for AI Agents
In src/index.ts around lines 3941 to 3960, standaloneSchema currently intersects
MacroContext directly which can leak non-schema keys; wrap the intersection in
SimplifyToSchema to narrow to schema-like keys. Replace the standaloneSchema
type from "Metadata['standaloneSchema'] & Schema & MacroContext" to
"SimplifyToSchema<Metadata['standaloneSchema'] & Schema & MacroContext>" (and
ensure SimplifyToSchema is imported/available) so non-schema keys like resolve
are not exposed.
|
LGTM |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/types.ts (1)
1726-1786: Fix duplicate type alias: GuardLocalHook (TS2300)Two declarations of GuardLocalHook exist in src/types.ts — delete the earlier definition (lines 1726–1786); the remaining definition begins at line 1787.
src/index.ts (1)
674-691: AOT vs non‑AOT inconsistency: headers.additionalProperties differs.Precompiled path sets additionalProperties: true, dynamic path uses !normalize. This can change behavior between aot=true/false. Align to a single behavior (suggest true, like precompiled).
- additionalProperties: !normalize, + additionalProperties: true,
♻️ Duplicate comments (3)
src/index.ts (3)
165-168: Re‑export newly imported public types (or drop unused import).You import IntersectIfObject and IntersectIfObjectSchema but don’t re‑export them; IntersectIfObject is also unused locally. If these are part of the public surface for 1.4.5, re‑export; otherwise remove IntersectIfObject. This mirrors the prior suggestion.
[ suggest_recommended_refactor ]
Apply this diff near the bottom where other type exports live:
+export type { IntersectIfObject, IntersectIfObjectSchema } from './types'To remove instead:
- IntersectIfObject, IntersectIfObjectSchemaRun to verify:
#!/bin/bash rg -n "export type \{[^}]*IntersectIfObjectSchema" -n rg -n "IntersectIfObject[^S]" src/index.tsAlso applies to: 8244-8289
3963-3966: Use SimplifyToSchema for standaloneSchema to avoid leaking non‑schema keys.Intersecting MacroContext directly may leak non‑schema keys (e.g., resolve). Wrap it.
- standaloneSchema: Metadata['standaloneSchema'] & - Schema & - MacroContext + standaloneSchema: Metadata['standaloneSchema'] & + Schema & + SimplifyToSchema<MacroContext>
4499-4502: Apply the same SimplifyToSchema guard in guard(...run) return type.Keeps standaloneSchema narrowed to schema‑like keys.
- standaloneSchema: Metadata['standaloneSchema'] & - Schema & - MacroContext + standaloneSchema: Metadata['standaloneSchema'] & + Schema & + SimplifyToSchema<MacroContext>
🧹 Nitpick comments (4)
test/types/lifecycle/soundness.ts (1)
1944-1985: Comment says “group” but code uses guard; align naming to avoid confusion.Either switch to
.group(...)if that’s intended, or update the comment.Apply (comment-only tweak):
-// Merge multiple group schema +// Merge multiple guard schema (comment corrected)example/a.ts (1)
1-17: Remove unused imports.
zodandreqaren’t used in this snippet.-import z from 'zod' -import { req } from '../test/utils'src/index.ts (2)
579-592: Filter out empty cookie validators to avoid passing undefined entries.Passing undefineds downstream is avoidable noise.
- validators: standaloneValidators.map((x) => x.cookie), + validators: standaloneValidators + .map((x) => x.cookie) + .filter(Boolean),
4049-4094: Normalize standalone response in group() like guard().guard() wraps string/TypeBox/Standard responses into {200: ...}; group() pushes response as-is. Normalize for consistency.
- standaloneValidator: !hasStandaloneSchema + standaloneValidator: !hasStandaloneSchema ? localHook.standaloneValidator : [ ...(localHook.standaloneValidator ?? []), - { - body, - headers, - query, - params, - cookie, - response - } + (() => { + const normalized = + !response + ? undefined + : typeof response === 'string' || + (response as any) && + (Kind in (response as any) || + '~standard' in (response as any)) + ? { 200: response } + : response + return { + body, + headers, + query, + params, + cookie, + response: normalized + } + })() ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
example/a.ts(1 hunks)src/index.ts(21 hunks)src/types.ts(4 hunks)test/types/lifecycle/soundness.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
test/types/lifecycle/soundness.ts (1)
src/index.ts (4)
Elysia(186-8168)Elysia(8170-8170)t(8172-8172)status(8213-8213)
src/types.ts (1)
src/index.ts (13)
RouteSchema(8237-8237)MaybeArray(8263-8263)OptionalHandler(8245-8245)AfterHandler(8275-8275)ErrorHandler(8247-8247)LifeCycleType(8250-8250)DocumentDecoration(8254-8254)BodyHandler(8244-8244)TransformHandler(8278-8278)MapResponse(8258-8258)AfterResponseHandler(8246-8246)BaseMacro(8259-8259)SingletonBase(8229-8229)
example/a.ts (2)
src/index.ts (3)
Elysia(186-8168)Elysia(8170-8170)t(8172-8172)src/type-system/index.ts (1)
t(657-657)
src/index.ts (2)
src/types.ts (11)
MergeSchema(729-768)MacroToContext(1011-1079)NonResolvableMacroKey(1653-1655)MaybeArray(261-261)OptionalHandler(1190-1205)AfterHandler(1207-1235)ErrorHandler(1369-1544)Prettify(301-303)AnyLocalHook(1591-1591)IntersectIfObjectSchema(717-727)UnwrapRoute(525-564)src/utils.ts (1)
mergeHook(220-334)
🪛 Biome (2.1.2)
src/types.ts
[error] 1787-1787: Shouldn't redeclare 'GuardLocalHook'. Consider to delete it or rename it.
'GuardLocalHook' is defined here:
(lint/suspicious/noRedeclare)
🪛 GitHub Actions: Build and Test
src/types.ts
[error] 1726-1726: TypeScript error TS2300: Duplicate identifier 'GuardLocalHook'.
[error] 1787-1787: TypeScript error TS2300: Duplicate identifier 'GuardLocalHook'.
🔇 Additional comments (7)
src/types.ts (3)
708-727: New IntersectIfObject and IntersectIfObjectSchema look correct.The conditional intersection behavior matches the intended schema-merge semantics.
If desired, we can tighten the constraint to avoid
any:-export type IntersectIfObject<A, B> = - A extends Record<any, any> +export type IntersectIfObject<A, B> = + A extends Record<keyof any, unknown> ? B extends Record<any, any> ? A & B : A - : B extends Record<any, any> + : B extends Record<keyof any, unknown> ? B : A
1066-1076: MacroToContext intersection order change seems fine.The added intersection keeps recursive macro context accumulation intact.
528-529: Keep UnwrapRoute's Path default as '' — intentional and safe.ResolvePath treats '' as "no path parameters" (PathParameterLike requires a "/:/*" pattern), so UnwrapRoute's default yields {} for params and does not change inference for routes without path params; handler types use Path | undefined for handler call-site flexibility.
test/types/lifecycle/soundness.ts (3)
1887-1899: Strengthen the GET-schema test with an explicit body type check.Right now it returns
bodybut doesn’t assert the merged shape.[ suggest_optional_refactor ]
Apply:- .get('/user/:id', ({ body }) => body, { + .get('/user/:id', ({ body }) => { + expectTypeOf(body).toEqualTypeOf<{ id: number; name: 'lilith' }>() + return body + }, { body: t.Object({ name: t.Literal('lilith') }) })
1987-2027: Duplicate of the previous block.If intentional, add a note; otherwise remove to cut noise.
[ suggest_nitpick_refactor ]
2030-2045: Macro context inheritance check looks good.Asserting
userfrom upstream macro in the downstream macro is correct.src/index.ts (1)
1-8169: Local verification required — ripgrep returned no outputScan returned no output; cannot confirm whether:
- MacroContext['response'] is present in resolve positions
- standaloneSchema intersections are missing SimplifyToSchema
- headers validator uses different additionalProperties between AOT/dynamic
Run from the repository root and paste the outputs:
rg -n -C2 "resolve[^\\n]MacroContext\['response'\]" src/index.ts
rg -n -C1 "standaloneSchema:.&\sSchema\s&\s*MacroContext" src/index.ts
rg -n -C2 "getSchemaValidator\(cloned\.headers,[^)]+additionalProperties:" src/index.ts
| Schema & MacroContext, | ||
| Singleton & { | ||
| derive: Ephemeral['derive'] & Volatile['derive'] | ||
| resolve: Ephemeral['resolve'] & Volatile['resolve'] | ||
| resolve: Ephemeral['resolve'] & | ||
| Volatile['resolve'] & | ||
| // @ts-ignore | ||
| MacroContext['response'] | ||
| }, |
There was a problem hiding this comment.
Bug: resolve type incorrectly includes MacroContext['response'].
This should be MacroContext['resolve']; current form pollutes resolve with response shape.
- resolve: Ephemeral['resolve'] &
- Volatile['resolve'] &
- // @ts-ignore
- MacroContext['response']
+ resolve: Ephemeral['resolve'] &
+ Volatile['resolve'] &
+ // @ts-ignore
+ MacroContext['resolve']📝 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.
| Schema & MacroContext, | |
| Singleton & { | |
| derive: Ephemeral['derive'] & Volatile['derive'] | |
| resolve: Ephemeral['resolve'] & Volatile['resolve'] | |
| resolve: Ephemeral['resolve'] & | |
| Volatile['resolve'] & | |
| // @ts-ignore | |
| MacroContext['response'] | |
| }, | |
| Schema & MacroContext, | |
| Singleton & { | |
| derive: Ephemeral['derive'] & Volatile['derive'] | |
| resolve: Ephemeral['resolve'] & | |
| Volatile['resolve'] & | |
| // @ts-ignore | |
| MacroContext['resolve'] | |
| }, |
🤖 Prompt for AI Agents
In src/index.ts around lines 3934 to 3941, the resolve type is incorrectly
including MacroContext['response'] (and using a @ts-ignore); change that to
MacroContext['resolve'] so resolve combines Ephemeral['resolve'],
Volatile['resolve'], and MacroContext['resolve'] instead of the response shape,
and remove the unnecessary @ts-ignore; ensure the combined type remains an
intersection as intended.
src/index.ts
Outdated
| resolve: Ephemeral['resolve'] & | ||
| Volatile['resolve'] & | ||
| // @ts-ignore | ||
| MacroContext['response'] | ||
| }, |
There was a problem hiding this comment.
Same bug in guard() overload: resolve includes MacroContext['response'].
Mirror the fix here.
- resolve: Ephemeral['resolve'] &
- Volatile['resolve'] &
- // @ts-ignore
- MacroContext['response']
+ resolve: Ephemeral['resolve'] &
+ Volatile['resolve'] &
+ // @ts-ignore
+ MacroContext['resolve']📝 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: Ephemeral['resolve'] & | |
| Volatile['resolve'] & | |
| // @ts-ignore | |
| MacroContext['response'] | |
| }, | |
| resolve: Ephemeral['resolve'] & | |
| Volatile['resolve'] & | |
| // @ts-ignore | |
| MacroContext['resolve'] | |
| }, |
🤖 Prompt for AI Agents
In src/index.ts around lines 4161 to 4165, the guard() overload incorrectly
includes MacroContext['response'] in the resolve union; change that to
MacroContext['resolve'] (or the correct resolve member on MacroContext) and
remove the // @ts-ignore, so the resolve type becomes Ephemeral['resolve'] &
Volatile['resolve'] & MacroContext['resolve'] (or the appropriate existing key)
to mirror the fix applied elsewhere.
Bug fix:
Summary by CodeRabbit
Breaking Changes
Bug Fixes
Documentation
Tests
Chores