cozy / cozy-client

Document store and React components for the Cozy platform
MIT License
13 stars 13 forks source link

Realtime support for queryConnect #444

Open ptbrowne opened 5 years ago

ptbrowne commented 5 years ago

Connecting realtime to queries is a bit cumbersome.

componentDidMount () {
  this.startRealtime()
}

componentWillUnmount() {
  this.stopRealtime()
}

startRealtime() {
    this.realtime.on(doctype, 'created', this.handleRealtime)
    this.realtime.on(doctype, 'updated', this.handleRealtime)
    this.realtime.on(doctype, 'removed', this.handleRealtime)
}

stopRealtime() {
  this.realtime.unsubscribeAll()
}

handleRealtime() {
  this.props.collection.fetch()
}

Should it be possible to do

queryConnect({
  apps: {
    query: client => client.all("io.cozy.apps"),
    realtime: true
  }
})

?

y-lohse commented 4 years ago

I've started working on this in https://github.com/cozy/cozy-drive/pull/2035 but there are some API choices to make before integrating this in cozy-client.

1. Should queries be updated in realtime on a case by case basis?

…or is it ok if it's all queries or none? I don't really see a use case where we would want some queries to be "live" and others not, so forme the answer is no.

2. What should the API look like?

I suggest something like in the PR in drive, eg. a <RealTimeQueries type="io.cozy.files" /> component that handles real time notifications when it's in the render tree, and unsubscribes when it's unmounted. I like it because it's a very react way of doing things, and the component can be dropped at a very high level in the tree if the whole app should use the realtime, or it can be included in some views only.

Another option would be a call to client, eg. client.enableRealtime('io.cozy.files'). It's more or less the same except the unsubscription part involves a bit more effort as you need to use willUnmount or useEffect etc.

(Technically we could have a useRealtime hook as well, but it wouldn't return anything..? I don't think this makes sense in terms of API design.)

Of course if we decide that we want to activate this feature on a query by query basis, passing a realtime: true option to the query as suggested in the first message of this thread is probably the way to go.

3. Should cozy-client have it's own realtime client?

Not sure this needs to be done in this PR, but so far we always register realtime as a plugin. Wouldn't it make sense for cozy-client to have it's own realtime instance?

ptbrowne commented 4 years ago
  1. One usecase could be performance reasons for example, some queries will maybe update at a rate that is too high. But since queries are a front-end concept, we cannot do this on the stack call for realtime. So this would only be a filtering at application level. I do not think we need it now.

  2. I think we should have 1st the API without React then the React API. But since the Realtime is a plugin, the call would be on the plugin (client.plugins.realtime) from the application point of view, or from a static method of the RealtimePlugin (RealtimePlugin.startSync(client, 'io.cozy.files')).

I think it is preferable to have it first as a standalone (without React) API since it is more versatile, and then we can use it in the React component.

  1. It would increase the size of the client by the size of the realtime plugin. How much would it add ? I think the core of cozy-client is already a bit big, I am not sure that we want the realtime plugin to be included, it is very easy to instantiate if needed and it is only needed to do once.
y-lohse commented 4 years ago

It would increase the size of the client by the size of the realtime plugin.

It could be declared as a peer dependency to avoid this, but fair point. It was more of an ad'hoc thought anyway.