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

Deprecate QueryOptions and MutationOptions #207

Closed
Archmonger opened this issue Jan 5, 2024 · 1 comment · Fixed by #241
Closed

Deprecate QueryOptions and MutationOptions #207

Archmonger opened this issue Jan 5, 2024 · 1 comment · Fixed by #241

Comments

@Archmonger
Copy link
Contributor

Archmonger commented Jan 5, 2024

Ref: #113

Current Situation

use_query and use_mutation currently require QueryOptions and MutationOptions objects to configure their behavior. Adding an extra import just for configuration is pretty annoying.

Proposed Actions

Deprecate QueryOptions and MutationOptions in favor of a different interface.

def use_query(
    query: Callable[FuncParams, Awaitable[Inferred]] | Callable[FuncParams, Inferred],
    args: Sequence | None = None,
    kwargs: MutableMapping | None = None,
    /,
    postprocessor: AsyncPostprocessor | SyncPostprocessor | None = None,
    postprocessor_kwargs: MutableMapping | None = None,
    thread_sensitive: bool = True,
):
    ...

This interface does pose a new challenge. Type hints for args and kwargs will no longer receive auto-complete support due to Python type hint limitations. However, this seems to be worth the trade off. Especially since it's likely this type hint limitation will eventually get resolved.

An alternative is to use unpacking (for example use_query(... , *args, **kwargs)), however, this would result in our configuration options, such as thread_sensitive and postprocessor, stepping into the bounds of **kwargs. As a result, that is unfortunately not a good solution since every config option we add has the potential to break a user application.

@Archmonger
Copy link
Contributor Author

@rmorshea When you have time can you provide your opinion on this suggested API change?

Will be folding it in to the next major release of ReactPy Django.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant