westonpace opened a new issue, #61:
URL: https://github.com/apache/arrow-js/issues/61

   ### Describe the enhancement requested
   
   There are a number of places that arrow's js package uses `instanceof`.  For 
example:
   
   ```
   // from table.ts in the Table constructor
   if (args[0] instanceof Schema) {
       schema = args.shift() as Schema<T>;
   }
   ```
   
   Unfortunately, this is quite brittle.  This will fail whenever a library 
ends up with multiple instances of the arrow library.  For example:
   
    * LanceDb currently uses arrow version 14.  If the calling user is using 
any other version of arrow then node will happily install two versions of arrow 
and the checks will fail. (also, these failures tend to be very opaque)
    * Even if the user is using the exact same version of arrow there are cases 
where two instances of arrow are created.  For example, our users using vite 
have run into https://github.com/vitejs/vite/issues/3910
   
   I would argue that some of the core types (at least `Schema` but maybe also 
`Table` / `Vector` / `RecordBatch` too) should be considered stable and users 
should be allowed to have different versions / instances.  We tried working 
around this in lancedb by declaring arrow a peer dependency and that solves the 
first bullet above but it's also somewhat unconventional (for javascript) and 
inconvenient for our users (they are forced to use whatever version of arrow we 
are using and when we change our arrow version it is a breaking change).
   
   This is not critical, we've worked around this by re-building the schema if 
the input from our user looks like a schema, but I wanted to see if other JS 
users/maintainers felt similarly and, if so, I might try and find some time to 
explore a fix.
   
   Some potential fixes:
   
    * Add methods like `isArrowSchema` and use those methods instead of 
`instanceof`
    * Customize the definition of `instanceof` by implementing 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/hasInstance
 on types like `Schema`.
    * Stick with the current but upstream methods like 
https://github.com/lancedb/lancedb/blob/dfc518b8fbc5920cf7dafd072335257be1e5d9c5/nodejs/lancedb/sanitize.ts#L491
 so that this kind of "schema rebuild" is maintained here in arrow.
   
   ### Component(s)
   
   JavaScript


-- 
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: issues-unsubscr...@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to