1.4.6 patch #1411
1.4.6 patch #1411
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds CHANGELOG v1.4.6 and broad type-surface changes (200-inline handling, union coercion, Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (TS)
participant Handler as Route Handler
participant TS as TypeChecker
Dev->>Handler: define route using inline status(200, payload) or union response
Handler->>TS: infer Route['response'][200] vs inline payload / union
Note right of TS #e6f4ff: 200 inline allowed forms include:\n- declared union\n- concrete payload\n- ElysiaCustomStatusResponse<200,...>\n- BunHTMLBundlelike
TS-->>Handler: compile-time OK or error
sequenceDiagram
autonumber
participant Macro as MacroProvider
participant Core as Elysia Core
participant Utils as mergeDeep
Macro->>Core: provide localHook.detail (may include arrays)
Core->>Utils: mergeDeep(local, macro, { mergeArray: true })
Note right of Utils #fff4e6: Arrays are merged only when\noptions.mergeArray === true (default false)
Utils-->>Core: merged detail (arrays preserved/merged)
sequenceDiagram
autonumber
participant Build as build.ts
participant Pack as package.json
Build->>Pack: read package.json
alt 'elysia' in pack.dependencies
Build-->>Build: throw Error("Error can't be a dependency of itself")
else
Build-->>Build: continue build
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 (2)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
4-4: Version mismatch with CHANGELOG (1.4.6).Bump package.json version to 1.4.6 to align with the new release entry.
- "version": "1.4.5", + "version": "1.4.6",
🧹 Nitpick comments (5)
CHANGELOG.md (1)
1-5: Changelog is missing the BunHTMLBundlelike note.Call out the new BunHTMLBundlelike addition to make the entry complete.
Apply:
# 1.4.6 - 15 Sep 2025 Improvement: - strictly check for 200 inline status code - coerce union status value and return type + - add BunHTMLBundlelike to inline handler responsessrc/universal/types.ts (1)
210-223: Tighten BunHTMLBundlelike typing (optional).Consider narrowing
loader: anyto a string union (e.g., 'js' | 'jsx' | 'ts' | 'tsx' | 'css' | 'json' | string) and adding a brief JSDoc with source semantics for maintainers.- loader: any + // e.g. Bun/Esbuild-like loader identifiers + loader: 'js' | 'jsx' | 'ts' | 'tsx' | 'css' | 'json' | stringsrc/types.ts (3)
38-38: Use type-only import for BunHTMLBundlelikeAvoid pulling a runtime dependency for a type-only symbol.
Apply this diff:
-import { BunHTMLBundlelike } from './universal/types' +import type { BunHTMLBundlelike } from './universal/types'
1148-1149: InlineResponse: BunHTMLBundlelike is redundant given Record<any, unknown>Because BunHTMLBundlelike is structurally a Record<string, unknown>, adding it here doesn’t change assignability. If the goal is stronger inference for bundles, consider narrowing InlineResponse (eg, remove the broad Record<any, unknown>) or at least deduplicate the duplicate Record<any, unknown> entries in this union.
Would you like a follow-up PR to tighten InlineResponse without breaking users?
1150-1166: Tuple helper: document semantics (repeats T to the union’s cardinality, not a per‑member tuple)This is useful, but the name can be misread. Add a brief JSDoc to prevent misuse.
Apply this diff:
+/** + * Produces a fixed-length tuple whose length equals the number of distinct members in union T. + * Note: Elements are typed as T (repeated), not as the individual union members. + */ export type Tuple<T, A extends T[] = []> = TuplifyUnion<T>['length'] extends A['length'] ? [...A] : Tuple<T, [T, ...A]>;
📜 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 (8)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)package.json(1 hunks)src/error.ts(1 hunks)src/types.ts(5 hunks)src/universal/types.ts(1 hunks)test/types/lifecycle/soundness.ts(1 hunks)test/types/standard-schema/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
example/a.ts (1)
src/error.ts (1)
status(73-81)
test/types/lifecycle/soundness.ts (1)
src/error.ts (1)
status(73-81)
src/error.ts (2)
src/utils.ts (2)
InvertedStatusMap(621-625)InvertedStatusMap(628-628)src/index.ts (1)
InvertedStatusMap(8203-8203)
src/types.ts (2)
src/universal/types.ts (1)
BunHTMLBundlelike(210-223)src/error.ts (1)
ElysiaCustomStatusResponse(42-71)
🔇 Additional comments (7)
test/types/standard-schema/index.ts (1)
87-87: LGTM: negative case asserts single-response strictness.The ts-expect-error on returning 'a' correctly exercises the check.
package.json (1)
193-193: Confirm @types/bun downgrade intent.This changes from ^1.2.16 to ^1.2.12. If this is to avoid a regression, consider a comment or pin to the exact working version; otherwise, prefer staying current.
example/a.ts (1)
8-27: LGTM: example demonstrates 200-union inline status correctly.Matches the new type behavior and keeps the handler minimal.
test/types/lifecycle/soundness.ts (2)
2086-2121: LGTM: strict 200 inline status validation is exercised.Covers both valid and invalid shapes; good signal for the stricter check.
2124-2145: LGTM: union return-type coercion is validated.Ensures
status(200, ...)conforms to declared 200-union.src/error.ts (1)
45-51: Variance tweak on T improves subtyping for responses.Removing variance on
Tis aligned with allowing response to be a subtype of the declared return; tests cover this.If TS variance support changes across environments, confirm CI compiles with the configured TypeScript (currently ^5.8.3).
src/types.ts (1)
1067-1068: MacroToContext recursion looks correctThe extra intersection with ExtractOnlyResponseFromMacro and the recursive MacroToContext call keeps inference intact while honoring the recursion guard.
| | (Route['response'] extends { | ||
| 200: any | ||
| } | ||
| ? Route['response'][200] | ||
| ? | ||
| | Route['response'][200] | ||
| | ElysiaCustomStatusResponse< | ||
| 200, | ||
| Route['response'][200], | ||
| 200 | ||
| > | ||
| : unknown) | ||
| // This could be possible because of set.status |
There was a problem hiding this comment.
Unknown in the 200-branch union weakens type safety; use never instead
When Route['response'] lacks 200, the else branch contributes unknown, and unknown | X collapses to unknown, allowing any return value. That contradicts the “strictly check 200” objective.
Apply this diff:
- | (Route['response'] extends {
- 200: any
- }
- ?
- | Route['response'][200]
- | ElysiaCustomStatusResponse<
- 200,
- Route['response'][200],
- 200
- >
- : unknown)
+ | (Route['response'] extends { 200: any }
+ ? Route['response'][200] |
+ ElysiaCustomStatusResponse<
+ 200,
+ Route['response'][200],
+ 200
+ >
+ : never)📝 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.
| | (Route['response'] extends { | |
| 200: any | |
| } | |
| ? Route['response'][200] | |
| ? | |
| | Route['response'][200] | |
| | ElysiaCustomStatusResponse< | |
| 200, | |
| Route['response'][200], | |
| 200 | |
| > | |
| : unknown) | |
| // This could be possible because of set.status | |
| | (Route['response'] extends { 200: any } | |
| ? Route['response'][200] | | |
| ElysiaCustomStatusResponse< | |
| 200, | |
| Route['response'][200], | |
| 200 | |
| > | |
| : never) | |
| // This could be possible because of set.status |
🤖 Prompt for AI Agents
In src/types.ts around lines 1197 to 1208, the conditional union currently uses
unknown for the else branch which collapses the union and weakens type safety;
change the else branch from unknown to never so that when Route['response']
lacks a 200 key the union becomes never (preserving strict 200-only checks).
Update the conditional expression to return never instead of unknown and ensure
any surrounding types/aliases that depend on this branch still compile (adjust
generics or add an explicit cast if necessary).
| ErrorHandle extends MaybeArray<ErrorHandler<any, any, any>>, | ||
| GuardType extends GuardSchemaType = 'standalone', | ||
| AsType extends LifeCycleType = 'local', | ||
| AsType extends LifeCycleType = 'local' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Default generic changed to 'local' but JSDoc says '@default "override"'
Align the comment with the actual default to avoid confusion.
Apply this diff:
- /**
- * @default 'override'
- */
+ /**
+ * @default 'local'
+ */Run this to search for other mismatched defaults:
🏁 Script executed:
#!/bin/bash
rg -n -C1 "@default 'override'|@default \"override\"" src | sed -n '1,200p'Length of output: 87
Update JSDoc default to 'local'
JSDoc currently states @default 'override' but the generic default is 'local' (src/types.ts:1763-1765).
- /**
- * @default 'override'
- */
+ /**
+ * @default 'local'
+ */📝 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.
| AsType extends LifeCycleType = 'local' | |
| /** | |
| * @default 'local' | |
| */ | |
| AsType extends LifeCycleType = 'local' |
🤖 Prompt for AI Agents
In src/types.ts around line 1761, the JSDoc @default value incorrectly says
'override' while the generic default is AsType extends LifeCycleType = 'local';
update the JSDoc to @default 'local' so the documentation matches the actual
generic default, and run a quick search to ensure no other mismatched JSDoc
defaults remain for LifeCycleType.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/types.ts (2)
1196-1215: Strict 200 check: changeunknowntoneverin the non-200 branchUsing
unknowncollapses the union and weakens the guarantee when 200 isn’t declared. Switch toneverto truly “strict‑check” inline 200 status.- : unknown) + : never)#!/bin/bash # Verify no lingering 'unknown' else branch remains in InlineHandler 200 path rg -nP -C2 "Route\\['response'\\] extends \\{\\s*200: any\\s*\\}\\s*\\?[^:]+:\\s*unknown" src/types.ts
1763-1767: Fix JSDoc default forasGeneric default is
'local', but JSDoc says'override'. Align the docs./** - * @default 'override' + * @default 'local' */ as?: AsType#!/bin/bash # Find other mismatched defaults referring to LifeCycleType rg -n "@default 'override'|@default \"override\"" src/types.ts -n
🧹 Nitpick comments (4)
example/a.ts (2)
14-17: Avoid console noise in examplesDrop debug logs to keep the snippet focused.
.onBeforeHandle(({ auth, set }) => { - console.log(auth?.id) - console.log(set.status) })
19-19: No-op property access; make intent explicitAccessing
app['~Volatile']at runtime is a no-op and may trigger “no-unused-expressions.” Usevoidto be explicit.-app['~Volatile'] +void app['~Volatile']src/types.ts (2)
38-38: Use type-only import for interfacesPrevents accidental runtime import and keeps bundlers lean.
-import { BunHTMLBundlelike } from './universal/types' +import type { BunHTMLBundlelike } from './universal/types'
1139-1149: InlineResponse: dedupeRecord<any, unknown>entry
Record<any, unknown>appears twice; remove the duplicate. KeepingBunHTMLBundlelikehere looks good.type InlineResponse = | string | number | boolean | Record<any, unknown> | Response | AnyElysiaCustomStatusResponse | ElysiaFile - | Record<any, unknown> | BunHTMLBundlelike
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)src/types.ts(6 hunks)test/types/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (3)
example/a.ts (1)
src/index.ts (2)
Elysia(186-8170)Elysia(8172-8172)
test/types/index.ts (2)
src/index.ts (3)
Elysia(186-8170)Elysia(8172-8172)status(8215-8215)src/error.ts (1)
status(73-81)
src/types.ts (2)
src/universal/types.ts (1)
BunHTMLBundlelike(210-223)src/error.ts (1)
ElysiaCustomStatusResponse(42-71)
🔇 Additional comments (5)
example/a.ts (2)
3-5: LGTM: AuthData shape export and use ofsatisfiesInterface is minimal and correct for the example;
satisfieskeeps the literal type without widening.
8-13: Derive handler union looks correctHeader check ->
{ auth: null }else{ auth: { id: 1 } }aligns with the union the types aim to surface.test/types/index.ts (2)
2692-2707: LGTM: derive union propagation testCorrectly exercises
{ readonly id: 1 } | nullunion and the 401 early-return path.
2710-2725: LGTM: resolve union propagation testMirrors the derive case and validates the union inside
onBeforeHandle.src/types.ts (1)
1067-1077: MacroToContext recursion: good, but watch instantiation depthThe self-recursive merge is fine and guarded by
R['length']≤ 16; keep an eye on compile times in large macro stacks.Run a quick TS compile timing on the type-tests to ensure no noticeable regression.
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 (1)
src/index.ts (1)
774-779: Likely regression: config.detail array fields will overwrite instead of concat.With the new mergeDeep default, arrays in
this.config.detailvslocalHook.detailwon’t merge. If concatenation is still desired here (as it is in macro detail), pass{ mergeArray: true }.Apply:
- if (isNotEmpty(this.config.detail)) - localHook.detail = mergeDeep( - Object.assign({}, this.config.detail!), - localHook.detail - ) + if (isNotEmpty(this.config.detail)) + localHook.detail = mergeDeep( + Object.assign({}, this.config.detail!), + localHook.detail, + { mergeArray: true } + )
🧹 Nitpick comments (2)
example/a.ts (2)
7-11: Conflicting decorator shapes (string vs array).
thing.ais[]inaand''inb. Given decorate defaults to append (no override),b’s string wins. If the intent was to demonstrate array merge, adjust the example or use{ as: 'override' }.
19-20: Gate server start in example.Avoid binding port on import; gate behind
import.meta.main.Apply:
- .use(a) - .listen(3000) + .use(a) + +if (import.meta.main) { + b.listen(3000) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)package.json(2 hunks)src/index.ts(5 hunks)src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
src/index.ts (2)
src/types.ts (2)
MaybeArray(262-262)OptionalHandler(1217-1232)src/utils.ts (1)
mergeDeep(52-98)
example/a.ts (1)
src/index.ts (2)
Elysia(186-8178)Elysia(8180-8180)
🔇 Additional comments (2)
src/index.ts (2)
5504-5506: Good: array-merge enabled for macro detail.Explicit
{ mergeArray: true }here preserves prior behavior for macro-provideddetailarrays.
4151-4153: Type param addition LGTM.Using
BeforeHandle extends MaybeArray<OptionalHandler<...>>improves accuracy for handler unions.
| resolve: Ephemeral['resolve'] & | ||
| Volatile['resolve'] & | ||
| // @ts-ignore | ||
| MacroContext['response'] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Typo in types: resolve includes MacroContext['response'] instead of ['resolve'].
This widens Singleton['resolve'] with the wrong macro facet and can mislead TS inference.
Apply:
- resolve: Ephemeral['resolve'] &
- Volatile['resolve'] &
- // @ts-ignore
- MacroContext['response']
+ resolve: Ephemeral['resolve'] &
+ Volatile['resolve'] &
+ // @ts-ignore
+ MacroContext['resolve']Search for similar occurrences:
🏁 Script executed:
#!/bin/bash
rg -nP --type ts "resolve[^\\n]*MacroContext\\['response'\\]"Length of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- showing src/index.ts around lines 3928-3952 ----"
if [ -f src/index.ts ]; then
sed -n '3928,3952p' src/index.ts || true
else
echo "src/index.ts not found"
fi
echo
echo "---- searching for MacroContext['response'] (single- and double-quotes) ----"
rg -n --hidden -S "MacroContext\\['response'\\]" || true
rg -n --hidden -S 'MacroContext\["response"\]' || true
echo
echo "---- searching for MacroContext['resolve'] (single- and double-quotes) ----"
rg -n --hidden -S "MacroContext\\['resolve'\\]" || true
rg -n --hidden -S 'MacroContext\["resolve"\]' || true
echo
echo "---- searching for resolve lines referencing MacroContext ----"
rg -n --hidden -S "resolve[^\\n]*MacroContext" || true
echo
echo "---- searching for resolve: Ephemeral['resolve'] snippet ----"
rg -n --hidden -S "resolve: Ephemeral\\['resolve'\\]" || trueLength of output: 5313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- occurrences of MacroContext['response'] with context ----"
rg -n -C3 "MacroContext\\\['response'\\\]" src || true
echo
echo "---- occurrences of MacroContext['response'] (double-quote) ----"
rg -n -C3 'MacroContext\["response"\]' src || true
echo
echo "---- occurrences of resolve: ... & MacroContext['response'] ----"
rg -n -C3 "resolve[^\\n]*MacroContext\\\['response'\\\]" src || true
echo
echo "---- show specific surrounding blocks for the top matches ----"
for loc in \
"src/index.ts:3928" "src/index.ts:3935" "src/index.ts:3958" "src/index.ts:4160" "src/index.ts:4506" "src/types.ts:1206"; do
file=${loc%%:*}
line=${loc##*:}
if [ -f "$file" ]; then
start=$((line-6))
[ $start -lt 1 ] && start=1
echo
echo "---- $file lines $start-$((line+6)) ----"
sed -n "${start},$((line+6))p" "$file" || true
fi
doneLength of output: 2580
Fix type typo: use MacroContext['resolve'] for the resolve field
Widening Singleton['resolve'] with MacroContext['response'] is incorrect and breaks TS inference.
File: src/index.ts — lines ~3937-3940
- 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 3937 to 3940, the type union uses
MacroContext['response'] for the resolve field which is a typo; replace
MacroContext['response'] with MacroContext['resolve'] so the resolve property is
widened correctly (i.e., change the union to include MacroContext['resolve']
instead of MacroContext['response']) to restore proper TypeScript inference.
| Volatile['resolve'] & | ||
| // @ts-ignore | ||
| MacroContext['response'] | ||
| }, |
There was a problem hiding this comment.
Same typo: guard resolve uses MacroContext['response'].
Should be MacroContext['resolve'].
Apply:
- 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.
| 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 4165 to 4168, the guard for "resolve" incorrectly
references MacroContext['response']; change that reference to
MacroContext['resolve'] so the type union uses MacroContext['resolve'] instead
of MacroContext['response'], keeping the rest of the union (Volatile['resolve']
& MacroContext['resolve']) intact and remove the erroneous @ts-ignore if no
longer needed.
| const skipKeys = options?.skipKeys | ||
| const override = options?.override ?? true | ||
| const mergeArray = options?.mergeArray ?? true | ||
| const mergeArray = options?.mergeArray ?? false |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Breaking default: arrays no longer merge in mergeDeep() unless explicitly enabled.
Changing mergeArray default to false will flip prior concatenation semantics to overwrite at all call‑sites. This is likely to surprise downstream merges (eg, route detail.tags or other array fields) unless those sites now pass { mergeArray: true }.
Run to audit call‑sites that may need { mergeArray: true }:
🏁 Script executed:
#!/bin/bash
# Find all mergeDeep call sites and show nearby options usage
rg -nC3 --type ts '\bmergeDeep\s*\('
# Specifically flag sites missing "mergeArray"
rg -nP --type ts '\bmergeDeep\s*\([^\)]*\)' | rg -nPv 'mergeArray\s*:'Length of output: 6913
Breaking default: arrays no longer merge in mergeDeep() unless explicitly enabled.
Verified — many call sites invoke mergeDeep(...) without passing mergeArray, so arrays will now be overwritten. Add { mergeArray: true } where concatenation is intended, or restore the previous default.
- src/index.ts:1069
- src/index.ts:4029
- src/index.ts:4030
- src/index.ts:4662
- src/index.ts:4663
- src/schema.ts:948
- src/schema.ts:950
- src/schema.ts:953
- test/units/merge-deep.test.ts:9
- test/units/merge-deep.test.ts:14
- test/units/merge-deep.test.ts:50
🤖 Prompt for AI Agents
src/utils.ts around line 66: the current line sets mergeArray default to false
which breaks existing callers that expect arrays to be concatenated; restore the
previous behavior by changing the default to true (i.e. set mergeArray =
options?.mergeArray ?? true) so callers that omit options keep array-merge
semantics, or alternatively update all listed call sites to pass { mergeArray:
true } if you prefer explicit opt-in.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/types.ts (2)
1763-1767: JSDoc default mismatch forasGeneric default is
'local'but comment says'override'. Update the doc./** - * @default 'override' + * @default 'local' */
1198-1208: Else-branch should beneverto enforce strict 200-only inline responsesUsing
unknowncollapses unions and weakens the “strictly check 200” contract. Switch tonever.- : unknown) + : never)src/index.ts (2)
3919-3924: Fix typo: resolve widened with MacroContext['response'] instead of ['resolve']This incorrectly widens resolve and harms inference. Should be MacroContext['resolve'].
- resolve: Ephemeral['resolve'] & - Volatile['resolve'] & - // @ts-ignore - MacroContext['response'] + resolve: Ephemeral['resolve'] & + Volatile['resolve'] & + // @ts-ignore + MacroContext['resolve']
4147-4151: Same typo in guard hook: use MacroContext['resolve']Guard’s resolve facet also references MacroContext['response'].
- resolve: Ephemeral['resolve'] & - Volatile['resolve'] & - // @ts-ignore - MacroContext['response'] + resolve: Ephemeral['resolve'] & + Volatile['resolve'] & + // @ts-ignore + MacroContext['resolve']
🧹 Nitpick comments (8)
example/a.ts (1)
2-2: Remove unused import or wire it up
openapiisn’t used. Either remove it or show usage (eg,new Elysia().use(openapi()).listen()), otherwise it can fail lint/ts checks.-import { openapi } from '@elysiajs/openapi' +// import { openapi } from '@elysiajs/openapi'src/types.ts (4)
38-38: Useimport typefor type-only importAvoid emitting a runtime import; mark as type-only.
-import { BunHTMLBundlelike } from './universal/types' +import type { BunHTMLBundlelike } from './universal/types'
1139-1149: InlineResponse has a duplicateRecord<any, unknown>and new BunHTMLBundlelike — clean up
Record<any, unknown>appears twice; remove the duplicate.- Adding
BunHTMLBundlelikehere is fine.type InlineResponse = | string | number | boolean - | Record<any, unknown> + | Record<any, unknown> | Response | AnyElysiaCustomStatusResponse | ElysiaFile - | Record<any, unknown> | BunHTMLBundlelike
1150-1167: Tuple utilities — OK
LastOf/Push/TuplifyUnion/Tuplelook standard and bounded by usage. Consider brief doc comments for maintainability.
2231-2235: Behavior/comment drift in ExcludeElysiaResponseCode now uses
A & {}(distribution) but the comment still says “Intersect all union…”. Confirm intent and update the comment (or restoreUnionToIntersectif intersection is desired).src/index.ts (3)
167-170: Export UnknownRouteSchema from indexYou import and use UnknownRouteSchema but don’t re-export it from the index surface, while EmptyRouteSchema is exported. Expose UnknownRouteSchema alongside EmptyRouteSchema.
export type { ... - EmptyRouteSchema + EmptyRouteSchema, + UnknownRouteSchema } from './types'Also applies to: 8280-8290
5487-5490: Array merge can duplicate detail arrays (eg, tags)mergeDeep(..., { mergeArray: true }) appends arrays and can inflate duplicates. Dedup common fields after merge.
- localHook.detail = mergeDeep(localHook.detail, value, { - mergeArray: true - }) + localHook.detail = mergeDeep(localHook.detail, value, { + mergeArray: true + }) + // Deduplicate common array fields + if (Array.isArray((localHook.detail as any).tags)) { + (localHook.detail as any).tags = Array.from( + new Set((localHook.detail as any).tags) + ) + }
5717-5723: Unify InlineHandler generics: apply NoInfer to Decorator in .getOther methods use NoInfer; get is the outlier. Align for consistent inference.
- const Handle extends InlineHandler< - NoInfer<Schema>, - Decorator, - // @ts-ignore - MacroContext - > + const Handle extends InlineHandler< + NoInfer<Schema>, + NoInfer<Decorator>, + // @ts-ignore + MacroContext + >
📜 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 (7)
CHANGELOG.md(1 hunks)build.ts(1 hunks)example/a.ts(1 hunks)package.json(2 hunks)src/index.ts(25 hunks)src/types.ts(7 hunks)test/types/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (3)
test/types/index.ts (1)
src/index.ts (3)
Elysia(188-8169)Elysia(8171-8171)t(8173-8173)
src/index.ts (2)
src/types.ts (3)
UnknownRouteSchema(2045-2054)ResolvePath(291-293)OptionalHandler(1217-1232)src/utils.ts (1)
mergeDeep(52-98)
src/types.ts (2)
src/universal/types.ts (1)
BunHTMLBundlelike(210-223)src/error.ts (1)
ElysiaCustomStatusResponse(42-71)
🔇 Additional comments (11)
build.ts (1)
3-3: JSON import for Bun build script — LGTM.
Importing package.json directly keeps the check simple and fast. No issues here.test/types/index.ts (5)
2035-2035: onTransform(local): params should be empty — OKExpectation updated to
{}for local onTransform matches the new UnknownRouteSchema behavior. LGTM.
2096-2138: Global lifecycle params index signature — OKTyping params as
{ [name: string]: string | undefined }for global scope aligns with src/index overloads. LGTM.
2145-2187: Scoped lifecycle params index signature — OKSame treatment for scoped lifecycle is correct and consistent. LGTM.
2689-2723: derive/resolve union enrichment tests — OKThe new tests asserting
auth: {id: 1} | nullpropagation are sound and exercise the stricter 200-status path. LGTM.
2726-2756: transform inheritance semantics — OK
- Guarded transform not inheriting schema types (params -> {}, body -> unknown) looks correct.
- Scoped transform casting params to the index signature is correct.
LGTM.
src/types.ts (2)
2045-2054: UnknownRouteSchema export — OKPublic
UnknownRouteSchemadefaults match the lifecycle typing used in tests. LGTM.
1066-1077: MacroToContext recursion — OKRecursive inclusion of nested macro keys with depth guard is correct. LGTM.
src/index.ts (3)
1431-1440: Good shift to UnknownRouteSchema for transform hooksUsing UnknownRouteSchema with ResolvePath/BasePath (and standardized params for scoped/global) is a solid simplification and matches the PR goals.
Also applies to: 1465-1472
1290-1293: LGTM: standardized params shape for global/scopedConsistently typing params as { [name: string]: string | undefined } across lifecycle overloads is clear and predictable.
Also applies to: 2050-2053, 2420-2423, 2540-2543, 2824-2827, 7452-7455
6802-6806: Nice: strict 200-only inline WS response extractionThe Schema['response'] extends { [200]: any } ? ... check enforces the “200 inline status” rule for WS subscribe.
Please confirm ComposeElysiaResponse in ./types applies the same 200-only rule for HTTP inline handlers so the behavior is uniform.
| if ('elysia' in pack.dependencies) | ||
| throw new Error("Error can't be a dependency of itself") |
There was a problem hiding this comment.
Harden the self-dependency check (avoid runtime throw, remove magic string, clearer message).
'in' on pack.dependencies throws if dependencies is absent; also hardcoding 'elysia' is brittle. Use pack.name and guard with ?? {}.
Apply this minimal fix:
-if ('elysia' in pack.dependencies)
- throw new Error("Error can't be a dependency of itself")
+// Guard against missing sections and tie to the actual package name
+if (pack.name in (pack.dependencies ?? {})) {
+ throw new Error(`"${pack.name}" cannot be a dependency of itself`)
+}Optional: also prevent self‑reference in dev/peer/optional deps:
+const sections = [
+ pack.dependencies,
+ pack.devDependencies,
+ pack.peerDependencies,
+ pack.optionalDependencies
+] as const
+if (sections.some(s => s && (pack.name in s))) {
+ throw new Error(`"${pack.name}" cannot be listed as its own dependency`)
+}📝 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 ('elysia' in pack.dependencies) | |
| throw new Error("Error can't be a dependency of itself") | |
| // Guard against missing sections and tie to the actual package name | |
| const sections = [ | |
| pack.dependencies, | |
| pack.devDependencies, | |
| pack.peerDependencies, | |
| pack.optionalDependencies | |
| ] as const | |
| if (sections.some(s => s && (pack.name in s))) { | |
| throw new Error(`"${pack.name}" cannot be listed as its own dependency`) | |
| } |
🤖 Prompt for AI Agents
In build.ts around lines 5-6, the current self-dependency check uses the literal
'elysia' and uses the 'in' operator on pack.dependencies which will throw if
dependencies is undefined; replace that with a safe guard and use pack.name for
the package identity: obtain deps = pack.dependencies ?? {} and then check if
pack.name && (pack.name in deps) and throw a clearer error message like `throw
new Error(\`Package "${pack.name}" must not list itself as a dependency\`)`;
optionally repeat the same safe check for devDependencies, peerDependencies and
optionalDependencies (each using the ?? {} guard) to prevent self‑references
there as well.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/index.ts (2)
3920-3924: Fix typo: resolve widened with MacroContext['response'] (should be ['resolve']).This breaks resolve inference for grouped routes/macros.
Apply:
- resolve: Ephemeral['resolve'] & - Volatile['resolve'] & - // @ts-ignore - MacroContext['response'] + resolve: Ephemeral['resolve'] & + Volatile['resolve'] & + // @ts-ignore + MacroContext['resolve']Run to ensure no other occurrences remain:
#!/bin/bash rg -nP "resolve[^\\n]*MacroContext\\['response'\\]" -S
4147-4151: Same typo in guard overload: use MacroContext['resolve'], not ['response'].Currently widens resolve with response facet.
Apply:
- resolve: Ephemeral['resolve'] & - Volatile['resolve'] & - // @ts-ignore - MacroContext['response'] + resolve: Ephemeral['resolve'] & + Volatile['resolve'] & + // @ts-ignore + MacroContext['resolve']
🧹 Nitpick comments (3)
src/index.ts (1)
5732-5737: Make Decorator non‑inferable in GET handler for consistency.Other HTTP methods use
NoInfer<Decorator>; align GET to avoid accidental widening.Apply:
- const Handle extends InlineHandler< - NoInfer<Schema>, - Decorator, - // @ts-ignore - MacroContext - > + const Handle extends InlineHandler< + NoInfer<Schema>, + NoInfer<Decorator>, + // @ts-ignore + MacroContext + >example/a.ts (2)
2-2: Remove unused import.
openapiis not used; can trigger noUnusedLocals.Apply:
-import { openapi } from '@elysiajs/openapi'
16-21: Stale comments: these are now typed.The “ERROR IS HERE” notes are outdated given the new typing; delete to avoid confusion.
Apply:
- resolve: ({ - headers, // Record<string, string | undefined> <------- ERROR IS HERE - should be typed - body // unknown <------- ERROR IS HERE - should be typed - }) => { + resolve: ({ headers, body }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)src/index.ts(26 hunks)test/types/lifecycle/soundness.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (3)
test/types/lifecycle/soundness.ts (2)
src/index.ts (4)
Elysia(188-8184)Elysia(8186-8186)status(8229-8229)t(8188-8188)src/error.ts (1)
status(73-81)
example/a.ts (1)
src/index.ts (4)
Elysia(188-8184)Elysia(8186-8186)t(8188-8188)headers(1066-1074)
src/index.ts (2)
src/types.ts (6)
UnknownRouteSchema(2045-2054)ResolvePath(291-293)OptionalHandler(1217-1232)IntersectIfObjectSchema(718-728)MergeSchema(730-769)UnwrapRoute(526-565)src/utils.ts (1)
mergeDeep(52-98)
🔇 Additional comments (5)
src/index.ts (2)
1429-1441: LGTM: UnknownRouteSchema adoption for transform contexts.Cleaner, faster generics; correct params typing for scoped/global modes.
Also applies to: 1465-1472
5501-5505: LGTM: Merge detail arrays instead of overriding.
mergeArray: trueavoids clobbering tags/arrays from macros.test/types/lifecycle/soundness.ts (3)
2086-2121: LGTM: Strict 200 inline status typing test.Covers valid/invalid union members and enforces the declared 200 shape.
2124-2146: LGTM: Union coercion for 200 status.Validates return narrowing against unioned 200 schema.
2148-2172: LGTM: Macro resolve inherits guarded schema types.Good coverage for scoped guard -> macro context propagation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/units/merge-deep.test.ts (1)
84-88: Make the test assertive and fix spelling ("freezed" → "frozen").Change the test name and make the intent explicit with expect(...).not.toThrow().
File: test/units/merge-deep.test.ts:84
-it('handle freezed object', () => { - new Elysia() - .decorate('db', Object.freeze({ hello: 'world' })) - .guard({}, (app) => app) - }) +it('handle frozen object', () => { + expect(() => { + const db = Object.freeze({ hello: 'world' }) + const app = new Elysia() + .decorate('db', db) + .guard({}, (app) => app) + }).not.toThrow() +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)example/a.ts(1 hunks)src/utils.ts(2 hunks)test/a.test.ts(0 hunks)test/units/merge-deep.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- test/a.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- example/a.ts
- CHANGELOG.md
- src/utils.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/units/merge-deep.test.ts (1)
src/index.ts (2)
Elysia(188-8184)Elysia(8186-8186)
Improvement:
BunHTMLBundleLiketo Elysia inline handlerSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Tests
Chores