fix: prevent sibling arrays from sharing optionalsInArray index

[willemli]

Fix sibling array optional field cleanup collision

The Bug

When a schema has sibling arrays where one contains optional/nullable fields, the generated cleanup code for those fields gets attached to the wrong array's loop, causing runtime crashes.

Example that crashes:

t.Object({
  users: t.Array(t.Object({
    name: t.String(),
    avatar: t.Nullable(t.Object({ url: t.String() }))
  })),
  meta: t.Object({
    pagination: t.Array(t.Object({ page: t.Integer() }))
  })
})

Error: Cannot read properties of undefined (reading 'avatar')

The cleanup code if(ar0v[i].avatar===undefined) delete ar0v[i].avatar was being attached to the pagination array loop instead of the users array loop.

Root Cause

instruction.array was a primitive number that got copied by value when spreading {...instruction} during recursion. Sibling branches each got their own copy, so both arrays would allocate index 0.

The Fix

Split into two fields:

• currentArrayIndex - tracks which array context we're inside (for storing cleanup paths)
• nextArrayIndex: { value: number } - global counter wrapped in object so mutations are shared across spread copies

Impact for Elysia

Any Elysia route with a response/body schema containing sibling arrays where one has t.Nullable() or t.Optional() fields would crash at runtime. After this fix, the cleanup code correctly targets only its own array.

Testing

• Added regression tests for sibling arrays with optionals
• All 62 exact-mirror tests pass
• All 1425 Elysia tests pass

Summary by CodeRabbit

• Bug Fixes

  • Fixed incorrect handling of optional fields in sibling arrays, preventing unwanted cross-references between separate array structures in validation logic.

• Tests

  • Added comprehensive test coverage for sibling arrays with optional fields at various nesting levels to ensure correct validation behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR modifies the array index tracking mechanism in the mirror generation logic. The Instruction interface replaces a single array: number field with currentArrayIndex: number and nextArrayIndex: { value: number } to track array context per instance, preventing index collisions between sibling arrays during transformation.

Changes

Cohort / File(s)	Summary
Core Mirror Logic src/index.ts	Refactored array index tracking: replaced array field with currentArrayIndex (propagated during recursion) and nextArrayIndex (mutable counter for allocation). Updated all array handling paths in handleRecord, handleTuple, and mirror recursion; adjusted optional property indexing logic and initial state construction in createMirror.
Test Coverage test/sibling-arrays.test.ts	Added three test cases for sibling array handling: regression test for no cross-reference of optionals between sibling arrays in different objects, validation of independent optionals within sibling arrays at same depth, and nested arrays with multiple optional levels.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 Indices once tangled, siblings crossed their streams,
Now nested arrays dream their separate dreams,
With context and counters, each knows its place,
No collisions in sight—a well-ordered space! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main fix: preventing sibling arrays from sharing an optionalsInArray index counter, which directly addresses the root cause described in the PR objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/index.ts (1)

458-459: Consider wrapping the array case in a block to satisfy linter.

Biome flags this declaration as accessible from other switch clauses. While the break at line 494 prevents actual issues, wrapping the case body in braces is a quick fix.

🔎 Proposed fix

 		case 'array':
+		{
 			if (
 				schema.items.type !== 'object' &&
 				schema.items.type !== 'array'
 			) {
 				// ... existing code ...
 			}

 			const i = instruction.nextArrayIndex.value
 			instruction.nextArrayIndex.value++
 			// ... rest of case ...

-			break
+			break
+		}

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80ba8a6 and 4cc232a.

📒 Files selected for processing (2)

• src/index.ts
• test/sibling-arrays.test.ts

🧰 Additional context used

🧬 Code graph analysis (1)

test/sibling-arrays.test.ts (1)

test/utils.ts (1)

• isEqual (9-18)

🪛 Biome (2.1.2)

src/index.ts

[error] 458-459: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (7)

test/sibling-arrays.test.ts (1)

6-127: Solid regression test coverage for the sibling array collision bug.

The tests effectively cover:

1. Cross-object sibling arrays with nullable fields (the original crash scenario)
2. Same-depth sibling arrays with independent optionals
3. Nested arrays ensuring outer optionals aren't affected by inner array processing

The comment at lines 7-11 clearly documents the regression being tested.

src/index.ts (6)

64-71: Clean fix using a reference type for the shared counter.

The split into currentArrayIndex (value semantics for isolation) and nextArrayIndex (reference semantics for sharing) correctly addresses the root cause where sibling branches received the same index due to primitive copy-by-value during object spread.

---

114-123: Correct index capture and propagation in handleRecord.

The pattern of capturing the index before incrementing the shared counter, then passing it as currentArrayIndex during recursion, ensures proper isolation between sibling and nested array contexts.

---

143-160: Consistent index handling in handleTuple.

Follows the same capture-increment-propagate pattern as handleRecord, maintaining consistency across all array-like handlers.

---

374-402: The +1 offset is correct but the coupling is subtle.

The comment at line 375 helps explain this, but the relationship between storing optionals at currentArrayIndex + 1 here and retrieving them at optionalsInArray[i + 1] (line 476) relies on i being captured before the increment. This is correct but worth noting for future maintainers.

---

471-474: Correct propagation of array context during item recursion.

The currentArrayIndex: i ensures nested elements and their optionals are correctly associated with this array's cleanup code.

---

566-582: Correct initial state for the new index tracking fields.

Starting both at 0 with nextArrayIndex wrapped in an object ensures the first array allocation gets index 0 and the mutable counter is properly shared across all recursive calls.

[SaltyAom]

Closing as fixed by #26

Thanks for your contribution!