fix: assertArrayLength narrows to exclude optional undefined#43
Conversation
📝 WalkthroughWalkthrough
ChangesassertArrayLength nullable narrowing refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/assert/array-length/array-length.assert.ts (1)
16-29:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Remove the old overload declarations.
These overload signatures should have been removed as part of this refactor. Their presence conflicts with the new generic signature at lines 6-14, causing TypeScript to resolve calls ambiguously. For example, a call with
string[]would match both the new signature (line 6) and the overload at line 16, leading to unpredictable type narrowing.🔧 Remove the conflicting overloads
-export function assertArrayLength< - TArray extends unknown[], - const N extends number, ->( - value: TArray, - expectedLength: N, - message?: string, -): asserts value is TArray & ArrayOfLength<TArray[number], N>; - -export function assertArrayLength<const N extends number>( - value: unknown, - expectedLength: N, - message?: string, -): asserts value is ArrayOfLength<unknown, N>; - /** * Assert that an array has exactly the expected length, with type narrowing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assert/array-length/array-length.assert.ts` around lines 16 - 29, The assertArrayLength function has duplicate overload declarations that conflict with the new generic signature, causing ambiguous type resolution. Remove the two old overload declarations for assertArrayLength (the ones that appear after the new generic signature) and keep only the new generic signature that is already defined earlier in the file. These obsolete overloads are no longer needed and their presence creates conflicting type narrowing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/assert/array-length/array-length.assert.ts`:
- Around line 16-29: The assertArrayLength function has duplicate overload
declarations that conflict with the new generic signature, causing ambiguous
type resolution. Remove the two old overload declarations for assertArrayLength
(the ones that appear after the new generic signature) and keep only the new
generic signature that is already defined earlier in the file. These obsolete
overloads are no longer needed and their presence creates conflicting type
narrowing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 30ac3fa8-f95b-4e93-8eb8-0d68479a66da
📒 Files selected for processing (2)
src/assert/array-length/array-length.assert.tssrc/assert/array-length/array-length.test.ts
Summary by CodeRabbit
Refactor
assertArrayLengthto improve type narrowing for optional array properties, providing better TypeScript inference when arrays may be undefined.Tests