Incorrect wrapping of proxy objects on every render

[mfbx9da4]

How frequently does the bug occur?

All the time

Description

https://github.com/realm/realm-js/blob/master/packages/realm-react/src/useObject.tsx#L78 @takameyer

// Wrap object in a proxy to update the reference on rerender ( should only rerender when something has changed )
return new Proxy(object, {});

The intention of this code is that the reference will only change if the object has changed to play nicely with React's immutability conventions. However, the hook is not invoked only when the hook fires. It is invoked anytime the parent component renders. The component could be rendering because of a totally unrelated change to some other component state which will cause unnecessary Proxy wrapping, unnecessarily new references, break React's immutability conventions and contradicts the intention of this code.

Easier to explain with code

const initialObject = {};
function useObject() {
  return new Proxy(initialObject, {});
}

export default function App() {
  const [count, setCount] = useState(0);
  const object = useObject();
  useEffect(() => {
    // This effect will be invoked everytime the component
    // renders (eg click the button), even though the object
    // never actually changed
    console.log("object changed", object);
  }, [object]);

  return (
    <div className="App">
      <h1>{count}</h1>
      <button onClick={() => setCount((x) => x + 1)}>Click me</button>
    </div>
  );
}

https://codesandbox.io/s/modern-field-7ety98?file=/src/App.js:69-642

The fix here is to create the new proxy in the updateCallback.

I think a further improvement would be to have a singleton observable object with a single realm listener with a reference count referring to the number of active hooks using it. This would:

• Reduce the overhead of fetching a new object each time because if the object is already in memory you can reuse it
• Improve resilience by reducing potential gotchas with lots of realm listeners such as useObject inside FlatList throws Exception in HostFunction: Cannot create asynchronous query while in a write transaction #4375
• Using the same object via useObject in two different components will return the same reference and so works best with React's immutability tendencies.

Side issue: while the object cache and collection will make great strides to improving immutability, non-primitive values such as Dates, sets and dictionaries will not be immutably cached correctly. Are you aware of that?

Stacktrace & log output

No response

Can you reproduce the bug?

Yes, always

Reproduction Steps

No response

Version

main

What SDK flavour are you using?

Local Database only

Are you using encryption?

No, not using encryption

Platform OS and version(s)

ios

Build environment

Which debugger for React Native: ..

Cocoapods version

No response

[takameyer]

@mfbx9da4 Good catch. Really appreciate the feedback. We will investigate this as time allows and get back to you when we have something in review.

[takameyer]

@mfbx9da4 We have a good idea for fixing the proxy wrapper. That being said, can you please make a separate issue for:

Side issue: while the object cache and collection will make great strides to improving immutability, non-primitive values such as Dates, sets and dictionaries will not be immutably cached correctly. Are you aware of that?

We would also appreciate an example where this is not being handled correctly 🙂 Thanks so much for your feedback!

[mfbx9da4]

@takameyer what do you make of my singleton object proposal? Is there somewhere better to discuss that?

[takameyer]

@mfbx9da4 I think it's worth looking into. We have discussed various ways we could improve or notification systems. I think making a feature request would be a good way to invoke more discussion.

[maxencehenneron]

Is there any progress on this? In my app, I do a CPU-intensive operation using the data stored in the database. I tried the following:

const result = useMemo(() => {
   console.log('starting CPU intensive operation');
   return intensiveOperation(object);
}, [object]);

but it seems like this if fired every time the component state changes, not when the realm "object" dependency changes.

Is it what's described in this ticket?

[tomduncalf]

It looks like the same thing that is described @maxencehenneron, yes. There's a PR #4519 with a fix for this issue which we are reviewing right now.

[sdandois]

Hi, I noticed that useQuery has the same problem.

Is there a previous version where this works correctly? The README file caught my attention:

useQuery

Returns Realm.Results from a given type. This Hook will update on any changes to any Object in the Collection and return an empty array if the Collection is empty. The result of this can be consumed directly by the data argument of any React Native VirtualizedList or FlatList. If the component used for the list's renderItem prop is wrapped with React.Memo, then only the modified object will re-render.

const Component = () => {
  // ObjectClass is a class extending Realm.Object, which should have been provided in the Realm Config.
  // It is also possible to use the model's name as a string ( ex. "Object" ) if you are not using class based models.
  const collection = useQuery(ObjectClass);

  // The methods `sorted` and `filtered` should be wrapped in a useMemo.
  const sortedCollection = useMemo(() => collection.sorted(), [collection]);

  return (
    <FlatList data={sortedCollection} renderItem={({ item }) => <Object item={item}/>
  )
}

That is not working as expected, all items in my list are being re-rendered.

[sdandois]

As a workaround, I've implemented a caching hook on top of the ones provided by @realm/react:

export const createRefCache = object => {
  return new Proxy(
    {},
    {
      get: function (target, property) {
        return object[property];
      },
      set: function (target, property, value) {
        object[property] = value;
      },
    },
  );
};

export const useCachedObject = (object, otherDeps = []) => {
  const [counter, setCounter] = useState(0);
  const forceRender = () => setCounter(n => n + 1);

  // eslint-disable-next-line react-hooks/exhaustive-deps
  const memoObject = useMemo(() => createRefCache(object), [counter, ...otherDeps]);

  useEffect(() => {
    const listener = (obj, changes) => {
      if (changes.deletions.length || changes.insertions.length || changes.modifications.length) {
        forceRender();
      }
    };
    memoObject.addListener(listener);

    return () => {
      memoObject.removeListener(listener);
    };
  }, [memoObject]);

  return memoObject;
};

Let me hear what you think about it!

[takameyer]

This has been fixed in @realm/react@0.4.0

[mordechaim]

@takameyer
I'm not sure what I'm missing, but I clearly still observe the issue described in #4913 for both useQuery and useObject.

Android 10

"react-native": "^0.70.5",
"realm": "^11.2.0",
"@realm/react": "^0.4.1"

[dimonnwc3]

I still experience issue for both useQuery and useObject

[channeladam]

Argh, same. Just stumbled upon this issue after creating #5264 .