Allow Django ORM usage within components

[Archmonger]

• 79

[rmorshea]

While I understand the aversion towards loading things from the database in effects, this is actually very common and generally recommend in React. That's because you don't want to block rendering while you're reading data, and you want to keep side effects like writing data to the database, inside a use_effect hook.

Typically, the way people handle this in react is to create a hook like this

data, loading, error = use_query(query)

if loading:
    return loading_spinner()
if error:
    return error_message(error)

return the_actual_view(data)

Where the implementation of use_query looks something like:

def use_query(query):
    data, set_data = use_state(None)
    loading, set_loading = use_state(True)
    error, set_error = use_state(None)

    @use_effect
    async def load_data():
        set_loading(True)
        try:
            set_data(fetch_from_database(query)
        except Exception as error:
            set_error(error)
        finally:
            set_loading(False)

    return data, loading, error

An example of such a hook can be found in the popular GraphQL client Apollo.

My recommendation would be to create a hooks that functions in a similar way to those from Apollo (i.e useQuery and useMutation) rather than allowing people to make queries in the main body of their components instead of in effects.

[Archmonger]

I believe the use_orm hook should live alongside this implementation. If the user intentionally doesn't mind the blocking behavior of ORM queries, I say let them have it.

For example, in Conreq's circumstance, there's a lot of situations where I'm absolutely reliant on ORM queries for each render. Having it built into the component vastly simplifies the design.

Also, frankly, the Django ORM doesn't work well in effects either due to similar protection schemas.

[rmorshea]

I guess I understand that. However, given that loading and mutating database resources should generally be done in effects, we ought to give people an easier way to do that since it's tedious to write out the loading/error/success states every time you need to access the DB in a component.

[Archmonger]

The Django ORM doesn't work well in effects either due to similar protection schemas.

Also, I'm about 80% certain we can't wrap the Django ORM in a use_orm hook due to those same protections. We very likely would either need heavy monkey patching on the ORM, or upstream PRs on django

There is a chance Django 4.1 will help reduce limitations enough for use_orm to exist, but that is yet to be seen. Given that Andrew Godwin hasn't had time to dedicate to Django OSS lately, I believe it will be a significant period of time before a real fix exists.

[rmorshea]

Ok, I don't think I understood the issue here. The fact that we're running in an event loop at all makes Django unhappy. Would something like this work to avoid the SynchronousOnlyOperation error?

@component
def sample():

    @use_effect
    @async_to_sync  # <-- I'm thinking that this might make Django happy in the thread_sensitive=True mode?
    async def do_orm_work():
        Model.objects.all()
        ...

[Archmonger]

I haven't tested the decorator directly on the effect, but it certainly does work when used inside the effect.

[rmorshea]

Also, is it the case that the SynchronousOnlyOperation only happens when a query set is executed? If so, then there might be a decent way to do this. I'm thinking the interface for queries (haven't thought about create/update operations yet) might be:

data, loading, error = use_query(Model.objects.all())

Inside use_query we would force the QuerySet to execute inside an effect wrapped in async_to_sync.

[Archmonger]

I will need to double check at what point during execution the exception is raised. I will get back to you on that.

[Archmonger]

And, if the exception is caused when .all() is called, we might be able to get away with an interface like this:

def use_query(query: Callable, *args, **kwargs):
    data, set_data = use_state(None)
    loading, set_loading = use_state(True)
    error, set_error = use_state(None)

    @use_effect
    async def load_data():
        set_loading(True)
        try:
            # Will return an async callable if the value is already async. 
            # If the value is sync, then `database_sync_to_async` will be invoked
            set_data(convert_to_async(query)(*args, **kwargs))
        except Exception as error:
            set_error(error)
        finally:
            set_loading(False)

    return data, loading, error

data, loading, error = use_query(Model.objects.all, example_val, value=other_example)

Perhaps we make the name even more generic, such as use_callable?

This interface can't cleanly be expanded to edit, update, and creation though... I think we should push this hotfix PR forward until Django's async ORM is released.

[rmorshea]

We'd take an approach similar to Apollo's useMutation hook to handle ORM object creation and updating.

If this is truly blocking we can push forward, but ultimately I'd like to remove this functionality in favor of hooks. We should brainstorm the interface for these hooks in a separate issue.

[Archmonger]

That useMutation API frankly looks quite ugly. That would be a really tough bullet for Django developers to swallow.

[Archmonger]

Is it worth pushing out this PR just to enable Django ORM behavior to be similar to the other frameworks IDOM supports?

If not, I'll close this PR.

[Archmonger]

If we're going to have to back this out at some point, I think it might be more realistic to just document how to use use_effect with database_sync_to_async as a temporary solution.

I don't think we should merge unless we can agree it's beneficial to keep this in long-term.

As mentioned, I personally think it might be beneficial for IDOM to allow this behavior, despite it not being a good programming pattern.

If a component is...

1) 100% DB dependent 2) and it is acceptable that failure to perform the DB query results in the component not rendering

... then I think we may as well allow this to happen, but simply document that it is most likely a bad idea.

If you disagree then this PR can be closed.