Fix Reorder.Group to accept union array types#3649
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR fixes a TypeScript-only issue with
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User Code\n<Reorder.Group values={items} onReorder={setItems}>"]
B["External Type Cast\nReorderGroup\nValues extends any[]"]
C{"TypeScript\ninfers Values"}
D["string[]"]
E["number[]"]
F["string[] | number[]\n(new - previously broken)"]
G["Values[number]\nextracts element type"]
H["Internal ReorderGroupComponent\nProps<V, TagName>\n(unchanged)"]
I["ReorderContext\nregisterItem / updateOrder"]
A --> B
B --> C
C --> D & E & F
D & E & F --> G
G -->|"V = Values[number]"| H
H --> I
Last reviewed commit: a25d58a |
|
|
||
| export const ReorderGroup = /*@__PURE__*/ forwardRef(ReorderGroupComponent) as < | ||
| V, | ||
| Values extends any[], |
There was a problem hiding this comment.
Consider
unknown[] over any[] for the constraint
Values extends any[] technically allows any to satisfy the constraint unchecked. Using Values extends unknown[] is marginally stricter and prevents any from silently widening through — a common best-practice for generic utilities where you don't need the escape hatch any provides.
| Values extends any[], | |
| Values extends unknown[], |
This is backwards-compatible: all existing concrete types (string[], number[], MyObject[], string[] | number[]) satisfy unknown[] just as they satisfy any[].
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
HardikDewra
left a comment
There was a problem hiding this comment.
Smart type-level fix. The original generic V on Reorder.Group meant passing a union array type like number[] | string[] would infer V as number | string, losing the array-level union distinction. By switching to Values extends any[] and using Values[number] for the item type while keeping Values as the array type for onReorder, TypeScript preserves the full union and setItems works correctly without type errors. The test demonstrates the exact use case - a state typed as number[] | string[] now works without needing to widen the type.
Summary
Fixes #2905
Reorder.Group'svaluesprop was typed asV[]with a single generic element typeV, which prevented passing union array types likenumber[] | string[]. TypeScript would inferV = numberfrom the first union member and then rejectstring[].Cause: The external type signature used
Vas the element type, sovalues: V[]couldn't representnumber[] | string[](a union of arrays is not the same as an array of unions).Fix: Changed the external type assertion to be generic over the full array type (
Values extends any[]) instead of just the element type. Thevaluesprop now acceptsValuesdirectly, andonReorderreturnsValues, so union array types flow through correctly. The internal implementation is unchanged — it derives the element type asValues[number]where needed.This is a backwards-compatible change: existing code using
string[],number[], orMyObject[]continues to work exactly as before.Test plan
useState<number[] | string[]>withReorder.Group— verifies the TypeScript compilation and renderingyarn build)🤖 Generated with Claude Code