Hook Cleanup

[Archmonger]

Description

This PR covers any oddities I discovered while integrating the new hooks with Conreq.

• Allow use_mutation to have refetch=None, as the docs suggest is possible.
• use_origin hook to return the browser's location.origin
• Fix flaky test_use_query_and_mutation test (Key presses require at least 100ms between keyup/keydown on GH actions)
• Logging for use_mutation/use_query errors (currently silent, which is very hard to debug)
• Document use_mutation's reset()
• Document use_query requires a Model or QuerySet return (due to enforcing no lazy queries)
• Document interface on all APIs
• Documentation misc cleanup and styling

Checklist:

Please update this checklist as you complete each item:

• Tests have been included for all bug fixes or added functionality.
• The changelog.rst has been updated with any significant changes, if necessary.
• GitHub Issues which may be closed by this PR have been linked.

[Archmonger]

@rmorshea I'm detecting something extremely fishy about how use_query operates.

If I manually remove refetch=... on the two use_mutation calls, somehow the Django model (items.data) performs another ORM call on the next component render... without IDOM ever re-fetching via get_items_query 😱

You can test this by removing the two refetch=... parameters, then manually toggling the checkboxes in Chrome. You'll see the TodoList visually add/remove elements despite no refetch=... being specified.

This might be indicative of a bigger problem with how we expect Django model queries to operate.

But before I go down the debugging rabbit hole, I'd like you to confirm my findings are not just a simple mistake.

[Archmonger]

Did some more debugging and determined there is no issue.

toggle_item_mutation changes the value of item.done by reference, which indirectly updates the state without running refetch. Theoretically, this means the displayed item states can possibly not be 1:1 with the database. But this should be resolved by end-users via adding a refetch=... if needed.

Unrelated note, deferred_fields are not equivalent to lazy fields. All model fields are lazy, but individual fields can also be set as deferred. So I am altering the fetch logic to accommodated for this.

[rmorshea]

Since IDOM uses Google's style guide for docstrings, let's stick to that for consistency. That can be done in a separate PR though.

I've also been wondering whether we should add a parameter to use_query that is False by default but which, if set to True, would remove the restriction that the return type be a Model or QuerySet. We can just create an issue for this right now, but I suspect someone is going to have a performance issue that would best be resolved by limiting what fields are returned.

[Archmonger]

Agreed that there should be an issue to modify use_query to remove restrictions on QuerySet and Model. Although the vast majority of Django users will use the built-in Django ORM, not all users do. SQLAlchemy and encode/orm are infrequently used with Django on large-scale projects.

I don't believe any other ORM has the same sync/async context protection as Django. That issue is unique to Django due to how all queries are required to be lazy. Truthfully, I really wish Django core would just give us a Model.lazy toggle, or a Model.execute function so we can drop this awkwardness entirely.

---

Regarding the docstrings, I've never been thrilled about declaring variable types via docstring. It's a bad practice that's been repeated for years (since Python 3.5+) due to Python 2 common practices. It's not DRY since it unnecessarily duplicates type-hint information.

The more proper method can be seen in mkdocstrings, where it automatically interprets Python type hints and cross-references that with docstring variable names. I'd eventually like to transition our docs to use that, but unfortunately that task is blocked.