Possible solution for exhaustive deps

[bimusiek]

What, How & Why?

Possible solution for exhaustive deps

This closes #6259

☑️ ToDos

Test it, but I cannot build the package as realm/bindgen is not published on NPM registry and not sure if this is something I have to build or I simply do not have access to.

Btw, would be amazing to have README also for contributors, would save A TON of time to figure out the inner workings around buildtools, scripts and ways you have the repo setup.

[kraenhansen]

Thanks for taking the time to look into this 👍

I cannot build the package as realm/bindgen is not published on NPM registry

The @realm/bindgen package is vendored in as a submodule of the repository: https://github.com/realm/realm-js/tree/main/packages/realm/bindgen/vendor has realm-core which links to https://github.com/realm/realm-core/blob/master/package.json.

Unfortunately our docs on building is a bit outdated since the major changes we did in v12, but at least some of the instructions here should be applicable: https://github.com/realm/realm-js/blob/main/contrib/building.md#cloning-the-repository

Specifically, you need to initialize submodules after cloning the repo:

git submodule update --init --recursive

After that, you should be able to simply run npm install in the root and then run npm test in the packages/realm-react package. If that's not possible, we have something we need to fix 🙂

[bimusiek]

@kraenhansen I have written one test for the new way of using useQuery. Please take a look.
Not all tests are passing as it looks like I don't have access to some secret keys you guys use in your CI workflow.

Same with my local test run:

    FetchError: request to http://localhost:9090/api/admin/v3.0/auth/providers/local-userpass/login failed, reason:

I tried to setup docker container, but then again there are some AWS keys I would need (probably to offload docker to AWS?). I did not play with it longer to make it work as it was not crucial for me to improve useQuery.

Also, there was another missing dependency with RealmProvider that I commited, but if it is wrong then tell me and I will add eslin-disable-next-line so it won't popup again.

[bimusiek]

I have published package to @hernas/realm-react so I am able to easily test it in our project and everything looks good now.

I had to export createUseQuery and createUseObject due to the way our app is written. We have extensive Realm initialisation process in place, to make sure it handles all the quirks and issues from past few years of using Realm, so we don't want to switch to createRealmContext where we pass the Realm.Configuration.

I don't think it is something bad. Now consumers of this library, can still use their own Realm initialisation code and start using @realm/react with it.

[kraenhansen]

Thanks again for taking the time to look into this. While I'm really happy you've found a way to solve the immediate issue, I'm also afraid that the added overload could cause a bit of confusion. I'm also not a super fan of the fact that the type becomes the third argument to the hook.

If I understand correctly, this overload is because the eslint rule expects deps to be the second argument.
Could we make this overload take just two arguments, the first being an object with query and type fields and the second being the deps?

An alternative solution would be to create a createQueryHook<T>(type: string) function, which would return a useQuery(query: QueryCallback<T>, deps?: DependencyList): Realm.Results<T>, they users could export a bunch of these to query their data:

export const usePersons = createQueryHook<Person>("Person");
export const useDogs = createQueryHook<Dog>("Dog");
// ... and so on, one for every class in their schema

// `createQueryHook` could even have an overload for class-based models, like some of our other APIs:
export const usePersons = createQueryHook(Person);

// And in a consuming component:
const twenty = 20;
const teenagers = usePersons(persons => persons.filtered("age < $0", twenty), [twenty]);

[bimusiek]

@kraenhansen Interesting idea, this would work even better.

Looks much cleaner to me.
And in our codebase we could have something like ImporterAuthenticationModel.useHook which would be simply createQueryHook(ImporterAuthenticationModel).

And as we already have our own BaseModel that all models extends from to add new functionalities to the Realm models, we can easily get hooks working then.

Let me get on that, just to be sure, something like this:

const createQueryHook = prepareCreateQueryHook(useRealm)
const useRealmDogs = createQueryHook <Dog>("dogs");
const useRealmOwners = createQueryHook(OwnerModel); // where OwnerModel extends Realm.Object

But then, you want to keep legacy useQuery hook as it was? And export additional new createQueryHook / prepareCreateQueryHook?

[kraenhansen]

Yep. Something like that. Perhaps the "prepare create" becomes a bit verbose, but you've got the gist of it. I'd like to get @takameyer's and @elle-j's thoughts on the above.

BTW: To work around the issue you're having with the Realm having to be opened outside of the provider, it might make sense for us to create a simple component (let's call it RawRealmProvider or RealmInstanceProvider) taking the Realm instance as a prop instead of a config, and use that from the current RealmProvider or perhaps simply overload the existing providers interface to take either props from config or a single realm prop. That way you don't need to call a prepareCreateQueryHook, but you can just use the defaults exported by the package.

[takameyer]

I really like what was proposed. I'll just present my spin on it.

Creating a createModelHooks method would be an straightforward step that we could implement right now and reap the benefits thereof.

My idea would be to make this exportable on the default RealmProvider and also add it to the return object of createRealmContext.

The createModelHook could return

import {createModelHooks} from '@realm/react';

// or
import {createModelHooks} from '../MyRealmContext';

class Person extend Realm.Object{} //some class based model

const [usePersonById, usePerson] = createModelHooks(Person);

Each hook returned would be bound to the realm and model and be an extension on useObject and useQuery respectively.

[bimusiek]

From our perspective as users of the library, I would love to be able to use as much barebone functions as we can. We already have our own RealmProvider that creates a context for us and we would love to stick with what we have, as it is quite tailored for our needs. We support from one codebase Electron, React Native but also web, which we cannot use Realm in.

Another long term way of doing this, would be to have realm context creation ability to accept list of models you want to work with in React.

Then we could have something like:

enum SchemaName {
    Dog = 'Dog',
    Cat = 'Cat'
}

abstract class RealmModel {
    static schemaName: SchemaName;

    static get() {
        console.log('get');
    }
}

class Dog extends RealmModel {
    static schemaName = SchemaName.Dog

    static woof() {
        console.log('woof');
    }
}

class Cat extends RealmModel {
    static schemaName = SchemaName.Cat

    static meow() {
        console.log('meow')
    }
}

type SchemaWithModels = {
    [SchemaName.Cat]: typeof Cat;
    [SchemaName.Dog]: typeof Dog
};

function createReactUtilities<Model extends typeof RealmModel>(models: Model[]) {
    type SchemaNameHooks = {
        [K in SchemaName as K extends string ? `use${K}` : never]:
        (callback: (query: SchemaWithModels[K]) => void, deps: any[]) => void
    }

    return models.reduce((hooks, model) => {
        return {
            ...hooks,
            [`use${model.schemaName}`]: (callback: any) => {
                callback(model);
            }
        }
    }, {}) as SchemaNameHooks;
}

const utils = createReactUtilities([Dog, Cat]);
utils.useCat((query) => {
    query.get();
    query.meow();
}, [])
utils.useDog((query) => {
    query.get();
    query.woof();
}, [])

Playground Link Here

This is just proof of concept, so there has to be more work & thought put into it. But theoretically it is possible to infer proper types and have hooks created. In the example above, query has type Cat in useCat and type Dog in useDog

[kraenhansen]

@bimusiek again - we really appreciate your engagement here!
No-one knows the ideal patterns for using our SDK better than our users ...

I've created #6284 to track the proposal to add the creation of these class-aware hooks. We've discussed your concrete proposal on the team and we found the overload a bit too envolved (expecially the fact that the type is the third argument is a deal-breaker for us).

As I see it, you're welcome to either

1. adopt this to the alternative of adding an overload taking an options object (with type and query) or alternatively
2. close this PR and create another one for @realm/react create class-aware hooks #6284 (in which case we should probably iron out the API beforehand - as we value the time your putting into this).
3. close this PR and have us implement a fix for either Improve useQuery arguments so react-hooks/exhaustive-deps recognise missing deps #6259 or @realm/react create class-aware hooks #6284 as it gets to the top of our backlog.

Would this either of those ways forward work for you?

[elle-j]

Adding the following for context and discoverability:

• React has had an issue open regarding this for a while (and a more recent one).
• They also had two proposed solutions (PR1, PR2) that were accidentally closed.
  • For now, it doesn't look like they plan on customizing the additionalHooks option.

[bimusiek]

@kraenhansen I understand! :)

I am gonna close this PR then and simply wait for you guys to find the best solution that works for everyone. We will stick with our package for now, until there is new version of @realm/react with the fixes for exhaustive deps, in whatever form they will appear.

And I would like to say, that RealmJS is going through amazing improvements and as the early users we are quite excited to be part of the ecosystem. You are doing great job with the React integration and overall developer experience.