OP-Engineering / op-sqlite

Fastest SQLite library for react-native by @ospfranco
MIT License
598 stars 41 forks source link

Cannot spread item object #65

Closed TommysG closed 8 months ago

TommysG commented 8 months ago

Describe the bug I have this code:

const { rows } = await db.executeAsync(query, [
    id
]);

const data = [];

for (let i = 0; i < rows?.length; i++) {
  const item = rows.item(i);
  data.push({
    ...item,
    properties: JSON.parse(item.properties || '[]')
  })
}

return data;

Even though the item has many properties, when returning the data array the only property is properties. I tried with Object.assign and i received this error Exception in HostObject::set for prop 'properties': Unknown JSI ArrayBuffer to variant value conversion, received object instead of ArrayBuffer.

Versions:

ospfranco commented 8 months ago

The objects returned are not regular JS Objects, but rather HostObjects, some operations won't work.

As for trying to assign any object to a property, unfortunately, that won't work. The object is trying to be cast into an ArrayBuffer because storing an object is not possible on the C++ side. On the C++ side properties need to be stored and cast to C++ types. Mostly to prevent race conditions and de-allocation between the JS Runtime and C++.

Basically, not a bug, but rather a limitation of HostObjects.

Also FYI, sometimes {...item, blah: 'foo'} gets transpiled to Object.assign(item, {blah: 'foo'}, so you are not creating a new object and that's why you are getting this error. So you really want to create a new object first and then spread the rest.

I will add a better error for people who encounter the same issue.

TommysG commented 8 months ago

Ok I got it. So the only way is creating a new object with all the properties returned from the item object and assign them to the new object by referencing them directly? I'm trying to migrate from react-native-sqlite-storage and I use the spread operator thing most of the times in my code that's why I'm asking.

BTW even if I create a new object and spread the rest into the new object, logging the new object the result will be empty or without the rest properties.

ospfranco commented 8 months ago

You probably need something like a shallow clone that copies all the 1st level dependencies and then just assign the parsed properties

let obj = {}
shallowClone(results._array[0], obj)
obj.properties = JSON.parse(obj.properties)
francescotescari commented 4 months ago

You probably need something like a shallow clone that copies all the 1st level dependencies and then just assign the parsed properties

let obj = {}
shallowClone(results._array[0], obj)
obj.properties = JSON.parse(obj.properties)

Isn't doing a shallowClone defying the whole purpose of using a HostObject in the first place? You have to copy all the rows metadata and values into plain JS object, which probably ends up using more memory and time than just returning a plain JSI object, right? Would still love an option to opt out from HostObjects: some people (like me) would be ok with paying a bit of a higher price for the JS object creation if that means that we don't break the lang semantics (like HostObjects do). Just giving feedback, as this can become a deal breaker. We will now evaluate performance with shallow clone vs alternatives (rn-quick-sqlite, expo sqlite). Thanks for you work, really appreciated!

ospfranco commented 4 months ago

You can use exec raw which will return an array without the keys it will be faster than expo and quick. Otherwise quick and expo are the same besides API differences, not much left to optimize.

ospfranco commented 4 months ago

Also, adding properties to host objects has been added as a feature on the latest versions. You are better off reading the docs than commenting on old issues.