POC for discussion #1
Conversation
| const subscriptions = new Map<number, QueryOptions>() | ||
| const { next: nextId } = sequence() | ||
|
|
||
| function execute(): void { |
There was a problem hiding this comment.
Something to call out here. This isn't async. And I want to keep it that way so that when used with sync functions there isn't an automatic delay. I think we can update this to support async actions by using Promise functions.
| return reactive({ | ||
| ...toRefs(query), | ||
| unsubscribe: () => { | ||
| query.unsubscribe() | ||
|
|
||
| if(channel.subscriptions.size === 0) { | ||
| deleteChannel(action, parameters) | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
This is a pattern you'll see a couple times. You cannot spread a reactive object or you'll lose the reactivity. But you can call toRefs and spread that into a new reactive.
Using this so that I can add logic to the unsubscribe function at each level.
There was a problem hiding this comment.
I haven't implemented intervals or refreshing or anything like that. This is just cache a value based on queries existing.
| [Symbol.dispose]: () => { | ||
| query.unsubscribe() | ||
| }, |
There was a problem hiding this comment.
I think this is all we need for dispose to work. But was getting build errors when testing. Might be too new for vite/vitest.
There was a problem hiding this comment.
I was able to get this working in 91cfc4c
| export { | ||
| query, | ||
| useQuery, |
There was a problem hiding this comment.
library exports a default query and useQuery which is what I'd expect most implementations to use rather than actually calling createQuery
| export type QueryLifecycle = 'app' | 'route' | 'component' | ||
|
|
||
| export type QueryOptions = { | ||
| maxAge?: number, | ||
| lifecycle?: QueryLifecycle | ||
| } |
There was a problem hiding this comment.
Non of this is implemented.
There was a problem hiding this comment.
I'm super excited to see the progress you've made so far. Seeing things like the support for using makes me super excited for what our next iteration of this will look like in the end.
The feedback I have right now is mostly just half baked ideas around possibilities to simplify.
I'm concerned that it's a bit confusing to have 3 different syntaxes (createQuery, query, useQuery), each with their own intended use cases but not extremely clear in naming. Do you think it's feasible that we combine syntaxes?
Another simplification I'd like to explore is dropping the separation of channel and manager. It's been a bit since I've worked with the version at Prefect but the last couple iterations I've built only have a manager, which keeps subscriptions and subscribers.
There was a problem hiding this comment.
I would say they both need to be renamed lol, "utility" is just a dumping ground
| return reactive({ | ||
| ...toRefs(query), | ||
| unsubscribe: () => { | ||
| query.unsubscribe() | ||
|
|
||
| if(channel.subscriptions.size === 0) { | ||
| deleteChannel(action, parameters) | ||
| } | ||
| } | ||
| }) |
| useQuery: QueryComposition | ||
| } | ||
|
|
||
| const noop = () => undefined |
There was a problem hiding this comment.
why do we need this? can't we just delay execute until we have args?
Description
More than just the basics but covers all the pieces I think we need here. I think there are some opportunities for simplification for sure as this mostly follows the structure I've used before. The primary things I want to call out
createQueryCreates an instance of the query function and a query composition. The query function is designed to be used on its own in non component use cases. Including scaffolding for dispose which means you could do things like
The project exports a default
queryanduseQueryfunction to use.