fix: add null check for object properties#1649
Conversation
WalkthroughA guard clause was added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
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 (1)
src/schema.ts (1)
218-229: Logic issue: premature return inside loop.Line 228 returns
schema.elysiaMeta === metainside the for loop, causing the function to exit after checking only the first property. This appears incorrect for two reasons:
- Early exit: The loop terminates after the first iteration regardless of whether more properties need checking
- Wrong target: It checks
schema.elysiaMeta(already checked at line 192) instead ofproperty.elysiaMetaCompare with
hasAdditionalProperties(line 115), which returnsproperty.additionalProperties.🔎 Proposed fix
for (const key of Object.keys(properties)) { const property = properties[key] + if (property.elysiaMeta === meta) return true + if (property.type === 'object') { if (hasElysiaMeta(meta, property)) return true } else if (property.anyOf) { for (let i = 0; i < property.anyOf.length; i++) if (hasElysiaMeta(meta, property.anyOf[i])) return true } - - return schema.elysiaMeta === meta }
🧹 Nitpick comments (1)
src/schema.ts (1)
99-119: Consider adding similar guard inhasAdditionalProperties.While line 103 handles
patternProperties, an object schema withoutproperties,patternProperties, oradditionalPropertieswould still crash at line 105 when callingObject.keys(properties).🔎 Suggested guard
if (schema.type === 'object') { const properties = schema.properties as Record<string, TAnySchema> if ('additionalProperties' in schema) return schema.additionalProperties if ('patternProperties' in schema) return false + if (!properties) return false for (const key of Object.keys(properties)) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/schema.ts
🔇 Additional comments (1)
src/schema.ts (1)
216-216: LGTM - Guard prevents crash with Record schemas.The null check correctly prevents the TypeError when
propertiesis undefined (e.g., witht.Recordwhich usespatternProperties). This is consistent with similar guards inhasType(line 176) andhasProperty(line 257).
Bug:
hasElysiaMetacrashes withTypeError: undefined is not an object (evaluating 'Object.keys(properties)')when schemas contain t.Record.Why it happens:
The fix: Add a null guard before iterating:
if (!properties) return falseSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.