[realm/react] Account for new ObjectId refs used in the dependency list

[elle-j]

Since a BSON.ObjectId reference used for the useQuery() dependency list will change when evaluated, users would either experience unnecessary rerenders if using the object, or would have to rely on the string representation, or they could use a memoized ObjectId (this would prob still depend in its string representation however).

In general, it's good to be cautious when using e.g. objects or functions as dependencies as these references may change, so using myObjectId.toHexString() would suffice.

To prevent unintended side effects of using ObjectIds as a dependency, we could cache such references or try to handle those dependencies with some "custom custom" comparator (in that case we'd have to create our own since passing custom comparators are not used for useCallback(), unlike e.g. memo()), or go through the dep list and replace the Oids with their .toHexString().

[sync-by-unito]

➤ PM Bot commented:

Jira ticket: RJS-2786

[bimusiek]

I feel like the best way would be to ensure that unique ObjectID keep their references. So if we call ObjectID.createFromHexString or get the same ObjectID from Realm, those would be the same object.

At least, results coming from Realm (by either .objects() or .objectForPrimaryKey()) could keep the same reference if the ID did not change. That would be my expectation as the user of the realm/react lib.

The best solution would be React team allowing to put custom comparators globally for the whole project, but I understand why they won't do it as it could simply be overused. (facebook/react#14476 (comment))

Hard problem to solve to be honest. We had simple case of fetching list of transfers, then sub-component needed to fetch something additional so it used transfer._id.

And now because we need to use toHexString it simply looks weird:

  const idHexString = transferCollection?._id.toHexString();
  const items = useQuery<TransferCollectionItemModel>(
    (objects) =>
      objects.filtered(
        `transferCollection._id == $0`,
        idHexString ? Realm.BSON.ObjectId.createFromHexString(idHexString) : new Realm.BSON.ObjectId()
      ),
    [idHexString],
    TransferCollectionItemModel.schema.name
  );

So weird, that one of our devs changed it back to not cast to string, thus re-introducing the bug to the source code.
Now, we have patched the React typings to disallow using ObjectIDs in deps, but that is also smelly solution.

[elle-j]

I agree that it's probably unlikely that the React team will add custom comparators in that way. Thanks for all your suggestions and thoughts, that'll definitely help us when considering our approaches.

P.S. for your example code, to avoid the createFromHexString(), perhaps you could just work with the string in the dep. list for now (instead of also in the query).

  const id = transferCollection?._id;
  const items = useQuery<TransferCollectionItemModel>(
    // ... just work with the ObjectId in the query ...
    [id?.toHexString()],
    // ...
  );

[bimusiek]

Then the eslint rule for exhaustive deps won't work. :) And without this rule, I know that some of devs will forget to put all required deps there.

[elle-j]

Ah yeah! 👍

[kraenhansen]

I agree a valid approach would be for Realm to keep a weak cache of BSON.ObjectId and BSON.UUID that it has ever returned. We've thought about using a similar approach to ensure the same accessor object is returned when pulling an object from the database, to ensure equality checks work as expected. We've experimented a bit but havn't yet found a solution that we're satisfied with.

I just thought of an alternative approach to this specific problem: What if useQuery did mapped known object types (Realm.Object, BSON.ObjectId, BSON.UUID, etc.) into their primitive representation before passing them into the underlying useCallback call?

realm-js/packages/realm-react/src/useQuery.tsx

Line 89 in 5dab798

const queryCallback = useCallback(query, [...deps, ...requiredDeps]);