query
query copied to clipboard
feat(angular-query): added observable support
I've added a query$
property on the options of the query. It's a function that accepts a context
just like the queryFn
prop does and it returns an observable. It automatically unsubscribe to the observable if the abort signal is called.
@arnoud-dv how does this look?
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
query | ⬜️ Ignored (Inspect) | Visit Preview | Jan 17, 2024 10:21pm |
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 26de58096ad2a0a4e5bc600b666e33f8596ee7cb:
Sandbox | Source |
---|---|
@tanstack/query-example-angular-basic | Configuration |
@tanstack/query-example-react-basic-typescript | Configuration |
@tanstack/query-example-solid-basic-typescript | Configuration |
@tanstack/query-example-svelte-basic | Configuration |
@tanstack/query-example-vue-basic | Configuration |
☁️ Nx Cloud Report
CI is running/has finished running commands for commit 26de58096ad2a0a4e5bc600b666e33f8596ee7cb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.
📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 1 target
Sent with 💌 from NxCloud.
First thoughts:
Can we do this via an overload on queryFn
? In this implementation it is possible to pass a function to queryFn
and a function returning an observable to query$
at the same time. This isn't very intuitive IMO.
I'm not convinced we should accept values coming in from the observable after the first one. This behaviour is quite different from the other adapters where a value is received once. Didn't delve deep into this yet but I can imagine APIs aren't designed for this and a few bugs may result from this too. It also needs quite a lot of code (bundle size) for something that won't be used very much. We may choose to support it but I don't think injectQuery
would be the place, maybe something like injectStreamingQuery
and we may need changes in core for that. What do you think?
If we could simplify this a bit with an overload and support a single value emission only it will be a great addition.
Maybe we can rename it to observableFactory
. However, I did try to return the type of Observable
on queryFn
but there are 2 issues I came across with:
- The
defaultQueryOptions
function starts to break apart as it doesn't know it can accept an Observable and I can't really override that. Besides, there are lots of types that start breaking (including thequery-options
file) and I'm not really sure that rewriting all those types is worth the effort. It would have been if we could've supported it on the core package but it's out of scope at the moment. - In order to check for the proper return type of the
queryFn
, in case it supported an Observable, I would've had to execute the function in thecomputed
signal that creates thedefaultedOptionsSignal
and I believe that would be innecessary. I would rather let the client know it should perform the queries.
On the injectStreamingQuery
part of the discussion, I believe having an Observable
as an option allows you to control what flow you decide to pass down to the queryClient
. Observables, by definition can emit multiple times and I believe it would be a really bad DX to allow something to behave contrary to it's natural state.
That being said, we could try and just support observables only on the new inject function but I believe it's name would be confusing to say the least. However, given Angular's normal behaviour, the http client should just return a response once and it should just finalize. That's why I'm thinking this new function wouldn't bring too much value.
Can't wait for feedback on all this. Thanks!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 0.00%. Comparing base (
3a127bf
) to head (451c131
). Report is 376 commits behind head on main.
:exclamation: Current head 451c131 differs from pull request most recent head 26de580
Please upload reports for the commit 26de580 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## main #6726 +/- ##
==========================================
- Coverage 41.71% 0 -41.72%
==========================================
Files 179 0 -179
Lines 7007 0 -7007
Branches 1416 0 -1416
==========================================
- Hits 2923 0 -2923
+ Misses 3713 0 -3713
+ Partials 371 0 -371
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.