Divyanshu-s13 commented on PR #340: URL: https://github.com/apache/arrow-js/pull/340#issuecomment-3574205415
> Just to check. Originally, this pull request was just fixing a type but now it's also changing logic. Was the original patch not enough (as you noticed after writing a test case)? No. Writing test cases revealed that: Changing the type definition broke Vector access patterns The solution required both type changes AND logic changes (method overrides in StructBuilder) The logic changes were minimal (method signatures) because the underlying setValue() already handled plain objects at runtime. So Yes - Logic Changes Were Necessary Because: ✅ Type-only approach broke existing code (destructuring, .toJSON() calls) ✅ Tests revealed the issue - without tests, we wouldn't have caught this ✅ Solution required overriding methods - not just changing type signatures ✅ Runtime behavior needed adjustment - the setValue() already supported plain objects at runtime, but TypeScript didn't know that without the method overrides. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
