Copilot commented on code in PR #377:
URL: https://github.com/apache/arrow-js/pull/377#discussion_r2771795780
##########
src/schema.ts:
##########
@@ -18,8 +18,32 @@
import { MetadataVersion } from './enum.js';
import { DataType, TypeMap } from './type.js';
+/** @ignore */
+const kSchemaSymbol = Symbol.for('apache-arrow/Schema');
+/** @ignore */
+const kFieldSymbol = Symbol.for('apache-arrow/Field');
+
export class Schema<T extends TypeMap = any> {
+ /**
+ * Check if an object is an instance of Schema.
+ * This works across different instances of the Arrow library.
+ */
+ static isSchema(x: any): x is Schema {
+ return x?.[kSchemaSymbol] === true;
+ }
+
+ /**
+ * Custom instanceof handler to work across different Arrow library
instances.
+ * @see
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/hasInstance
+ */
+ static [Symbol.hasInstance](x: any): x is Schema {
+ return Schema.isSchema(x);
+ }
Review Comment:
The new type checking functionality (Symbol.hasInstance and static is*
methods) lacks test coverage. Consider adding tests that:
1. Verify instanceof works across different library instances using
Symbol.hasInstance
2. Test the static is* methods (isSchema, isField, isDataType, etc.)
3. Test the exported helper functions (isArrowSchema, isArrowField, etc.)
4. Verify the behavior when objects don't have the marker symbol
These tests would ensure the cross-library instanceof functionality works as
intended and doesn't regress in future changes.
--
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]
