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

Add ability to do a one-off query #59

Closed
stubailo opened this issue Mar 30, 2016 · 7 comments · Fixed by #67
Closed

Add ability to do a one-off query #59

stubailo opened this issue Mar 30, 2016 · 7 comments · Fixed by #67
Assignees

Comments

@stubailo
Copy link
Contributor

Right now you can call watchQuery then stop once you get the data, but that's not optimal.

@stubailo stubailo added this to the newspring-production milestone Mar 30, 2016
@jbaxleyiii
Copy link
Contributor

@stubailo do you think it would be confusing if we have a query method on the client? watchQuery is more descriptive of what the method does for sure, just don't want to cause confusion between creating a query.

Counter point would be that query and watchQuery will return drastically different things so there is no point in unifying the calling method. It would do more harm than good. They both seem like accurate explanations of what the method does.

query should return a promise that resolves to be the full result of the query, even if it is a merged cache + remote hit.

@stubailo
Copy link
Contributor Author

Yeah I agree, these are separate methods, since one returns a fancy handle and the other will return a simple promise. IMO having both query and watchQuery methods is fine. Realistically, the end developer might not be using watchQuery much, since they will probably be using the view integration?

I think we should have consistent options where possible though, like whether to use the cache.

One important question - does query use watchQuery as the implementation, or does it do something else?

@jbaxleyiii
Copy link
Contributor

hmmm, if it does, it comes down to a simple sugaring of watchQuery with a promise and self cleanup. I think the initial run could be that exactly. I'm not sure of the performance gains by bypassing the watch methods internally?

It shouldn't be a lot of overhead?

What if I'm watching a query, and also want to do a direct query of what will boil down to the same queryId? If it's just sugaring, the deregistration would remove the watched query handle yes?

@stubailo
Copy link
Contributor Author

What if I'm watching a query, and also want to do a direct query of what will boil down to the same queryId?

Right now, if you do the same query twice, it will have two different query IDs, so this isn't a problem. I think the efficiency benefits of deduplicating queries and doing ref counts would be pretty minimal in the current architecture.

@jbaxleyiii
Copy link
Contributor

const queryId = this.idCounter.toString();
this.idCounter++;

duh James

Seems easy enough! @johnthepink can probably knock this out tomorrow then because he is 🚀

@fikriauliya
Copy link

fikriauliya commented Oct 12, 2016

Right now, if you do the same query twice, it will have two different query IDs, so this isn't a problem. I think the efficiency benefits of deduplicating queries and doing ref counts would be pretty minimal in the current architecture.

@stubailo it doesn't happen in the case of server prerendering whereby apollo store is pre-populated with query running on server. (#759)

The client request would starts from zero again, effectively overwrite the 0th query on Store.

screen shot 2016-10-11 at 5 10 29 pm

@helfer
Copy link
Contributor

helfer commented Oct 12, 2016

@fikriauliya Actually, we don't need to send the query store to the client when doing SSR. The client initializes its own queries, so any existing queries in the store are completely useless. Could you make a PR to remove the query store from the data that's sent to the client when doing SSR? That would be great! 🙂

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants