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

liveQuery fails with a query that has a singular result (like findRecord) #178

Open
coleyb opened this issue Apr 4, 2019 · 7 comments
Open

Comments

@coleyb
Copy link

coleyb commented Apr 4, 2019

Attempting to liveQuery a single record will fail

For example:

model({ schedule_id }) {
    return this.store.liveQuery(q => {
      return q.findRecord({ type: 'schedule', id: schedule_id })
    });
  },
@coleyb coleyb changed the title liveQuery fails with a query that has a singular result (like findRecord) liveQuery fails with a query that has a singular result (like findRecord) Apr 4, 2019
@tchak
Copy link
Member

tchak commented May 10, 2019

LiveQuery are backed by an array, so I am not surprised they fail with a singular result query. There is several strategies to making LiveQuery work with singe record results. Live result needs to hold on to something so it can't return just a null because how would you update a null?

  • Keep using a live array and in case of single record populate it with one record
  • return a LiveQueryResult on which one would access the value: Record | Record[] | null
  • do not return a result but a subscription – will be less ergonomic for use in templates

@tchak
Copy link
Member

tchak commented May 21, 2019

I think the first option is the right one. I will submit a PR.

@dgeb
Copy link
Member

dgeb commented May 29, 2019

One other option worth considering would be to safelist query ops that return collections and then error if an unrecognized op is passed. And then we wouldn't need to force singular results into arrays.

My main concern with changing the shape of the response is that usages like @coleyb's would still return an array to a model hook that is almost surely expecting a single result.

@tchak
Copy link
Member

tchak commented May 30, 2019

Well, I guess. I think in general, liveQuery is an abstraction in ember-data that may be confusing for users in the regard of what it actually does. Like the one on the store, it only runs the query through sources once, and then query local cache only. As mentioned in orbitjs/orbit#456 what people may expect is to subscribe to a particular query server side (like in firebase or graphql). I don't think we have the proper abstraction in orbit now to do this. But I digress :)

Going back to the issue at hands. I don't really like the idea of relying on inspecting query expression but I guess it follows the principle of less surprise...

I am OK with fixing current ember-data liveQuery implementation the way @dgeb thinks is the best and then focus on a better support for “live queries” in orbit itself if possible.

@dgeb
Copy link
Member

dgeb commented May 30, 2019

@tchak hehe ... I think you mean ember-orbit instead of ember-data in your previous comment ;)

Like the one on the store, it only runs the query through sources once, and then query local cache only.

I think this aspect of liveQuery is fine because live queries continue to have their content updated as it changes in their associated cache. Thus, you could set up sources that emit changes (such as firebase, sockets, SSE), connect them to your store, and have each liveQuery stay in sync.

My concern is properly representing a singular object / null result, and doing so consistently with collection-based results. Probably you're right that the best place to solve this is in orbit itself and to avoid anything ember-specific in the solution (like ArrayProxy).

This is one area where I need to take some time and try out different alternatives.

I don't really like the idea of relying on inspecting query expression but I guess it follows the principle of less surprise...

I don't really like this either, but that was the exact reason I suggested it. Another alternative would be to use ES Proxys to represent object results. However, that doesn't seem to answer the problem of object presence (when the query result is null).

@tchak
Copy link
Member

tchak commented Jul 7, 2019

I am more and more convinced that async iterators are the way to go for live query results. They are used for graphql subscriptions and apollo is moving to async iterators as public interface. Also unlike observables async iterators are natively supported on modern platforms.

@dgeb
Copy link
Member

dgeb commented Jul 8, 2019

I am more and more convinced that async iterators are the way to go for live query results.

@tchak I agree that async iterators have become the standard way to represent live collections. We should consider whether ES Proxys can nicely pair with them to represent different types of live results. I'd like to experiment with all of this in a separate (non-ember) package.

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

Successfully merging a pull request may close this issue.

3 participants