Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useObject will return referentially different objects for the same object #4521

Closed
mfbx9da4 opened this issue Apr 24, 2022 · 5 comments · May be fixed by #4544
Closed

useObject will return referentially different objects for the same object #4521

mfbx9da4 opened this issue Apr 24, 2022 · 5 comments · May be fixed by #4544
Assignees

Comments

@mfbx9da4
Copy link

mfbx9da4 commented Apr 24, 2022

How frequently does the bug occur?

All the time

Description

Given the immutability conventions of react you would expect that

const object = useObject('foo', 0)
const object2 = useObject('foo', 0)
assert(object === object2)

However this is not the case.

Referential equality is commonplace for the equivalent of useObject in all the most common state libraries such as redux, mobx and zustand. It's very common to use hooks instead of prop drilling and therefore natural to use useObject multiple times for the same object. This issue could produce potential gotchas for users such as unnecessary re-renders.

One solution would be to maintain an object cache, each object would only have a single realm listener. The cache entry would track the number of active hooks there are for the object via list of plain JS listener callbacks. When the component unmounts the listener would be removed from the list of listeners. When the list of listeners drops to 0 the cache entry would be purged.

This model would likely have some other benefits

A similar approach could be applied to useQuery

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
Copy link
Contributor

Sounds like a good enhancement. I did attempt something similar to this at the beginning of the hooks effort, but opted for the simpler approach. I do believe it's a good idea to share the cached objects between hooks calls when requesting the same data. We will get this planned and update as progress is made.

@mfbx9da4
Copy link
Author

mfbx9da4 commented Apr 25, 2022

I have a pretty clear idea of how to implement this. I could throw together an implementation quite quickly. The slower part will be testing. Perhaps I could make a PR to an upstream feature branch which we could collaborate on to fully test?

Would this be useful or would you prefer to tackle it all in house?

@takameyer
Copy link
Contributor

Would this be useful or would you prefer to tackle it all in house?

If you made a PR, it would help get the feature out faster. But of course, we would need some tests surrounding it. We are using jest and @testing-library/react-native to test how the hooks render in components. There are rendering tests for both useObject and useQuery. So one should either extend those, or create a new test file to test a particular feature.

As this package is quite new, we haven't really written any guides on how to contribute. I'll write a guide here and then transfer that into a contribution document.

realm-js is a monorepo that uses lerna to dynamically link dependencies. In the case of @realm/react, the realm library is linked into the package. Therefore, one must build realm in order to run the @realm/react tests. We have a guide for building realm, but I will provide an abridged version here.

I assume you are already setup to run a react-native project and are on a mac, so a good amount of the required pre-req steps are already accomplished. What is probably missing is cmake, which can be installed with:

brew install cocoapods cmake

That should be enough to get started. If you are making changes to c++ code then I recommend setting up ccache, but if not, feel free to skip that step.

The rest of the steps are as follows.

  • clone the project
git clone https://github.com/realm/realm-js.git
cd realm-js
  • load the external dependencies (ie realm-core)
git submodule update --init --recursive
  • install js packages and build realm
npm install
  • install and link all dependencies to @realm/react
npx lerna bootstrap --scope @realm/react --include-dependencies
  • navigate to @realm-react and run the tests
cd packages/realm-react
npm run test
  • optionally run the watcher and match to a specific test suite
npm run test -- --watch --testNamePattern "useQueryRender" --detectOpenHandles

That should be enough to get started. Just shout if you have any questions.

@takameyer
Copy link
Contributor

This has been resolved in @realm/react@0.4.0

@TommyTheTribe
Copy link

Issue is still present as #4544 has never been merged

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants