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

[meta] Support real-time updates via event subscriptions #541

Closed
josephsavona opened this Issue Nov 2, 2015 · 74 comments

Comments

Projects
None yet
@josephsavona
Contributor

josephsavona commented Nov 2, 2015

Realtime data in GraphQL is something that we and the community are actively exploring. There are many ways to achieve "realtime" or near-realtime updates: polling, "live" queries, or event-based approaches (more on these tradeoffs on the GraphQL blog). Furthermore, there are a variety of transport mechanisms to choose from depending on the platform: web sockets, MQTT, etc.

Rather than support any one approach directly, we would prefer to allow developers to implement any of these approaches. Therefore, we don't plan to create a RelayMutation-style API for subscriptions. Instead we're working create a "write" API that will make it easy for developers to tell Relay about new data (along the lines of store.write(query, data)). See #559 for more information.

For now, we recommend checking out @edvinerikson's relay-subscriptions module.

@skevy

This comment has been minimized.

Show comment
Hide comment
@skevy

skevy Nov 2, 2015

Contributor

Just fwiw, I've actually started implementing this. I'm only so far into it - but I just wanted to put here that I'm actively working on it.

Contributor

skevy commented Nov 2, 2015

Just fwiw, I've actually started implementing this. I'm only so far into it - but I just wanted to put here that I'm actively working on it.

@josephsavona josephsavona assigned Sakedo and unassigned Sakedo Nov 2, 2015

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Nov 2, 2015

Contributor

@skevy I'm not sure why, but I can't assign this to your directly - but thanks for the heads up, looking forward to this!

Contributor

josephsavona commented Nov 2, 2015

@skevy I'm not sure why, but I can't assign this to your directly - but thanks for the heads up, looking forward to this!

@skevy

This comment has been minimized.

Show comment
Hide comment
@skevy

skevy Nov 2, 2015

Contributor

@josephsavona it's cuz I'm not a collaborator. Silly Github.

Is anyone at FB working on this for OSS release? Or only for internal use

Contributor

skevy commented Nov 2, 2015

@josephsavona it's cuz I'm not a collaborator. Silly Github.

Is anyone at FB working on this for OSS release? Or only for internal use

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Nov 2, 2015

Contributor

Aha! No one is actively working on the OSS Subscriptions (the API described above) - so you won't conflict :-)

Contributor

josephsavona commented Nov 2, 2015

Aha! No one is actively working on the OSS Subscriptions (the API described above) - so you won't conflict :-)

@dallonf

This comment has been minimized.

Show comment
Hide comment
@dallonf

dallonf Nov 2, 2015

The "subscribe/dispose" API seems awfully imperative and fiddly to control... what about something like this where RelayContainers define subscriptions declaratively:

module.exports = Relay.createContainer(Story, {
  fragments: {
    story: () => Relay.QL`
      fragment on Story {
        text,
        likeCount
      }
    `,
  },
  subscriptions: (props) => [
    new StoryLikeSubscription({storyId: props.storyId})
  ]
});

dallonf commented Nov 2, 2015

The "subscribe/dispose" API seems awfully imperative and fiddly to control... what about something like this where RelayContainers define subscriptions declaratively:

module.exports = Relay.createContainer(Story, {
  fragments: {
    story: () => Relay.QL`
      fragment on Story {
        text,
        likeCount
      }
    `,
  },
  subscriptions: (props) => [
    new StoryLikeSubscription({storyId: props.storyId})
  ]
});
@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Nov 2, 2015

Contributor

@dallonf That's a great idea. Ultimately there has to be an imperative API somewhere: in Relay, that's Relay.Store. We do need imperative APIs for opening subscriptions and disposing them, but the API you described would be a great way to provide declarative access to those methods from containers.

Note that the query tree is static and props are unavailable: instead the API should use variables:

  subscriptions: variables => [...]
Contributor

josephsavona commented Nov 2, 2015

@dallonf That's a great idea. Ultimately there has to be an imperative API somewhere: in Relay, that's Relay.Store. We do need imperative APIs for opening subscriptions and disposing them, but the API you described would be a great way to provide declarative access to those methods from containers.

Note that the query tree is static and props are unavailable: instead the API should use variables:

  subscriptions: variables => [...]
@skevy

This comment has been minimized.

Show comment
Hide comment
@skevy

skevy Nov 2, 2015

Contributor

@dallonf yah some type of declarative API to wrap it is smart. 100% agree.

Contributor

skevy commented Nov 2, 2015

@dallonf yah some type of declarative API to wrap it is smart. 100% agree.

@dallonf

This comment has been minimized.

Show comment
Hide comment
@dallonf

dallonf Nov 2, 2015

@josephsavona Agreed, I would definitely need the subscribe/unsubscribe methods in some cases.

Although now I wonder if the RelayContainer is the right place for this... you'd still have to add this particular subscription to any component that requests story { likeCount }. And it kind of runs the risk of coincidence-driven-development where every component that uses likeCount will benefit from just one RelayContainer in the tree that defines a subscription - but remove that component, and suddenly your real time updates stop working!

I wonder if it would be possible to implement "live queries" on the client, so that whenever the current route contains likeCount, it automatically subscribes to every (opt-in) Subscription that could update likeCount... kind of like a fat query in reverse? This feels like one of those models which would reveal a lot of intractable edge cases as soon as you got in too deep to get back out 😛 . It certainly works for the given example of `StoryLikeSubscription, but the whole point of the Subscription thing seems to be to provide more flexibility than live queries allow. What sort of use cases would break this model?

dallonf commented Nov 2, 2015

@josephsavona Agreed, I would definitely need the subscribe/unsubscribe methods in some cases.

Although now I wonder if the RelayContainer is the right place for this... you'd still have to add this particular subscription to any component that requests story { likeCount }. And it kind of runs the risk of coincidence-driven-development where every component that uses likeCount will benefit from just one RelayContainer in the tree that defines a subscription - but remove that component, and suddenly your real time updates stop working!

I wonder if it would be possible to implement "live queries" on the client, so that whenever the current route contains likeCount, it automatically subscribes to every (opt-in) Subscription that could update likeCount... kind of like a fat query in reverse? This feels like one of those models which would reveal a lot of intractable edge cases as soon as you got in too deep to get back out 😛 . It certainly works for the given example of `StoryLikeSubscription, but the whole point of the Subscription thing seems to be to provide more flexibility than live queries allow. What sort of use cases would break this model?

@nodkz

This comment has been minimized.

Show comment
Hide comment
@nodkz

nodkz Nov 3, 2015

Contributor

@dallonf great idea. But I recommend use Hash for subscriptions like in fragments. In order to be able to have several subscriptions, and be able to get access to their state:

subscriptions: {
   toMasterNode: (variables) => new StoryLikeSubscription({storyId: variables.storyId}),
   toSlaveNode: (variables) => new StoryLikeSubscription2({storyId: variables.storyId}, {disabled: true}),
}

@josephsavona It would be great if we can get subscriptions status and ability to manipulate them in Component:

class Story extends React.Component {
  render() {
    return (
      <div>
         <p>{this.props.story.text}</p>
         <p>{this.props.story.likeCount}</p>
         { !this.relay.subscriptions.toMasterNode.connected ?
             <p>
                   {`Not connected (${this.relay.subscriptions.toMasterNode.state})`}
                   <button onClick={()=>this.relay.subscriptions.toMasterNode.connect()}>Enable</button>
             </p> :
             <p>
                   <button onClick={()=>this.relay.subscriptions.toMasterNode.disconnect()}>Disable</button>
             </p>
         }         
      </div>
    );
  }
}
Contributor

nodkz commented Nov 3, 2015

@dallonf great idea. But I recommend use Hash for subscriptions like in fragments. In order to be able to have several subscriptions, and be able to get access to their state:

subscriptions: {
   toMasterNode: (variables) => new StoryLikeSubscription({storyId: variables.storyId}),
   toSlaveNode: (variables) => new StoryLikeSubscription2({storyId: variables.storyId}, {disabled: true}),
}

@josephsavona It would be great if we can get subscriptions status and ability to manipulate them in Component:

class Story extends React.Component {
  render() {
    return (
      <div>
         <p>{this.props.story.text}</p>
         <p>{this.props.story.likeCount}</p>
         { !this.relay.subscriptions.toMasterNode.connected ?
             <p>
                   {`Not connected (${this.relay.subscriptions.toMasterNode.state})`}
                   <button onClick={()=>this.relay.subscriptions.toMasterNode.connect()}>Enable</button>
             </p> :
             <p>
                   <button onClick={()=>this.relay.subscriptions.toMasterNode.disconnect()}>Disable</button>
             </p>
         }         
      </div>
    );
  }
}

@F21 F21 referenced this issue Nov 4, 2015

Closed

Subscription support #49

@F21

This comment has been minimized.

Show comment
Hide comment
@F21

F21 Nov 4, 2015

What's the plan in terms of the network layer for this? I would imagine that for push notifications, we would need websocket or socket.io's fallback method of long-polling or flash.

Will we be able to have real-time notifications on a websocket connection and mutations and queries still happening over http?

F21 commented Nov 4, 2015

What's the plan in terms of the network layer for this? I would imagine that for push notifications, we would need websocket or socket.io's fallback method of long-polling or flash.

Will we be able to have real-time notifications on a websocket connection and mutations and queries still happening over http?

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Nov 4, 2015

Contributor

@F21 Because there are multiple approaches to pushing data from server to client, we will likely leave the implementation of sendSubscription up to the user. This would allow developers to, for example, use HTTP for queries/mutations and some other method for establishing subscriptions.

This is per the description:

Provide a stub implementation in RelayDefaultNetworkLayer which throws when subscriptions are requested

Contributor

josephsavona commented Nov 4, 2015

@F21 Because there are multiple approaches to pushing data from server to client, we will likely leave the implementation of sendSubscription up to the user. This would allow developers to, for example, use HTTP for queries/mutations and some other method for establishing subscriptions.

This is per the description:

Provide a stub implementation in RelayDefaultNetworkLayer which throws when subscriptions are requested

@pasviegas

This comment has been minimized.

Show comment
Hide comment
@pasviegas

pasviegas Nov 9, 2015

Contributor

It would be actually really cool if subscriptions had a fat query like mutations do :)

Contributor

pasviegas commented Nov 9, 2015

It would be actually really cool if subscriptions had a fat query like mutations do :)

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Nov 10, 2015

Contributor

@pasviegas Good idea! Unfortunately it isn't quite so simple. Subscription queries may execute on the server at any time after the subscription is opened, which means that the "tracked" (active) queries on the client can be different between executions. While we could allow users to define a fat query for subscriptions, it would mean that the data fetched by the subscription would depend on what had been queried when the subscription was first created. In other words, it would create a non-deterministic query that would be difficult to reason about.

Requiring a static subscription query makes it clear exactly what data will be refreshed when a subscription event occurs.

Contributor

josephsavona commented Nov 10, 2015

@pasviegas Good idea! Unfortunately it isn't quite so simple. Subscription queries may execute on the server at any time after the subscription is opened, which means that the "tracked" (active) queries on the client can be different between executions. While we could allow users to define a fat query for subscriptions, it would mean that the data fetched by the subscription would depend on what had been queried when the subscription was first created. In other words, it would create a non-deterministic query that would be difficult to reason about.

Requiring a static subscription query makes it clear exactly what data will be refreshed when a subscription event occurs.

@faceyspacey

This comment has been minimized.

Show comment
Hide comment
@faceyspacey

faceyspacey commented Dec 21, 2015

+1

@faceyspacey

This comment has been minimized.

Show comment
Hide comment
@faceyspacey

faceyspacey Dec 21, 2015

Once the API is down (perhaps in some pre-release form) I would love to figure out how to put this to use in the Meteor world. We have a client/server pubsub API that uses a custom Json format, DDP, to communicate LiveQuery updates to clients. There have been lots of scalability issues with LiveQuery and plans to move to a pre-write webserver layer event-based approach to push changes to clients, which my assumption is what Relay subscriptions would be all about.

The questions I'm interested in exploring are:

  • using Relay, would we even use our pubsub API anymore?
  • what about the DDP Json protocol?
  • or do we resolve just to the base websockets API Meteor uses?
  • exactly what that Meteor already offers would we reuse if anything?

As soon as I find the answers to these questions and have some base Relay subscriptions tools to use, I'd love to start implementing this for Meteor (along with something like Graffiti of course since Meteor uses Mongo). I think Meteor could be a great guinea pig given it's one of the longest standing most used solutions for the whole subscription + reactivity enchilada here. I personally don't even know of any other LiveQuery solutions besides rethinkdb which isn't in the same stages as meteor, which has been offering this as their bread and butter for approaching 4 years. Our community has a lot of developers that would be willing--rather, eager--to test this out. I also know Meteor Development Group (the company behind the framework) is seriously considering this route as well.

LiveQuery won't work for us anymore. GraphQL/Relay is really looking like the way forward to many in the Meteor community. Let me know what I can do.

faceyspacey commented Dec 21, 2015

Once the API is down (perhaps in some pre-release form) I would love to figure out how to put this to use in the Meteor world. We have a client/server pubsub API that uses a custom Json format, DDP, to communicate LiveQuery updates to clients. There have been lots of scalability issues with LiveQuery and plans to move to a pre-write webserver layer event-based approach to push changes to clients, which my assumption is what Relay subscriptions would be all about.

The questions I'm interested in exploring are:

  • using Relay, would we even use our pubsub API anymore?
  • what about the DDP Json protocol?
  • or do we resolve just to the base websockets API Meteor uses?
  • exactly what that Meteor already offers would we reuse if anything?

As soon as I find the answers to these questions and have some base Relay subscriptions tools to use, I'd love to start implementing this for Meteor (along with something like Graffiti of course since Meteor uses Mongo). I think Meteor could be a great guinea pig given it's one of the longest standing most used solutions for the whole subscription + reactivity enchilada here. I personally don't even know of any other LiveQuery solutions besides rethinkdb which isn't in the same stages as meteor, which has been offering this as their bread and butter for approaching 4 years. Our community has a lot of developers that would be willing--rather, eager--to test this out. I also know Meteor Development Group (the company behind the framework) is seriously considering this route as well.

LiveQuery won't work for us anymore. GraphQL/Relay is really looking like the way forward to many in the Meteor community. Let me know what I can do.

@ansarizafar

This comment has been minimized.

Show comment
Hide comment
@ansarizafar

ansarizafar Dec 22, 2015

@faceyspacey I would love to test your implementation.

ansarizafar commented Dec 22, 2015

@faceyspacey I would love to test your implementation.

@tonyxiao

This comment has been minimized.

Show comment
Hide comment
@tonyxiao

tonyxiao Dec 23, 2015

@faceyspacey We would love to explore how to use Relay / GraphQL together with Meteor as well, Let me know if you end up going down this route further. @qimingfang fyi.

tonyxiao commented Dec 23, 2015

@faceyspacey We would love to explore how to use Relay / GraphQL together with Meteor as well, Let me know if you end up going down this route further. @qimingfang fyi.

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Dec 31, 2015

Anyone I can coordinate with on trying to help out on this?

eyston commented Dec 31, 2015

Anyone I can coordinate with on trying to help out on this?

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 12, 2016

I'm going to try and take a stab at this. I checked with @skevy and he might start next week so I'll try and communicate anything I do in case it is useful.

I spent this afternoon looking at Relay mutation code. I have a few questions:

  1. Is it desirable to keep a central reference to all subscriptions? There is the RelayMutationQueue which holds all mutations, but from what I can tell this is required for:
  • queueing collisions
  • optimistic updates get re-run multiple times (if I understand the code) so a reference is required to all mutations
  • RelayContainer can check if there are pending mutations on a record

There might be other reasons I'm missing / not understanding.

I'm not sure if subscriptions would require something similar. It could be useful for visibility and maybe some kind of mass dispose. Either way it would be easy to add them to a central map someplace if desired.

  1. For writeRelayUpdatePayload, again, just a quick glance, it looks like it can be re-used for subscriptions. This sounds reasonable / expected? The only issue I saw was handleRangeAdd has an invariant on clientMutationID. Could that invariant be removed and the code with RelayMutationTracker only be run when clientMutationID is in the payload?

I haven't looked at the functionality of RelayMutationTracker yet -- todo list for tomorrow -- so this might answer itself.

  1. Finally, I looked at RxJS to try and familiarize myself with the lingo. Here is rough pseudo-code for Relay#subscribe:
  subscribe(subscription, callbacks) {
    // the RelaySubscriptionObserver class does two things:
    // - add an onNext for calling writeRelayUpdatePayload
    // - enforce all the observer rules / laws / etc
    const observer = new RelaySubscriptionObserver(subscription, store /* or wuteva */, callbacks);
    const request = new RelaySubscriptionRequest(subscription, observer);

    // coerce the return value from RelayNetworkLayer#subscribe into a disposable e.g. Thing#dispose()
    const disposable = createDisposable(RelayNetworkLayer.sendSubscription(request));
    observer.setDisposable(disposable);
    return observer.getDisposable();
  }

code for RelayNetworkLayer#sendSubscription would return a function that performs unsubscribe / dispose:

  sendSubscription(request) {
    const id = 1; // placeholder ...

    const handler = response => {
      if (response.id === id) {
        if (response.data) {
          request.onNext(response.data);
        } else if (response.error) {
          request.onError(response.error);
        }
      }
    });

    socket.on(`graphql:subscription:${id}`, handler);

    // subscribe
    socket.emit('graphql:subscribe', {
      id,
      query: request.getQueryString(),
      variables: request.getVariables()
    });

    return () => {
      // unsubscribe
      socket.off('graphql:subscription', handler);
      socket.emit('graphql:unsubscribe', {id});
    };
  }

Seem reasonable?

Thanks!

eyston commented Jan 12, 2016

I'm going to try and take a stab at this. I checked with @skevy and he might start next week so I'll try and communicate anything I do in case it is useful.

I spent this afternoon looking at Relay mutation code. I have a few questions:

  1. Is it desirable to keep a central reference to all subscriptions? There is the RelayMutationQueue which holds all mutations, but from what I can tell this is required for:
  • queueing collisions
  • optimistic updates get re-run multiple times (if I understand the code) so a reference is required to all mutations
  • RelayContainer can check if there are pending mutations on a record

There might be other reasons I'm missing / not understanding.

I'm not sure if subscriptions would require something similar. It could be useful for visibility and maybe some kind of mass dispose. Either way it would be easy to add them to a central map someplace if desired.

  1. For writeRelayUpdatePayload, again, just a quick glance, it looks like it can be re-used for subscriptions. This sounds reasonable / expected? The only issue I saw was handleRangeAdd has an invariant on clientMutationID. Could that invariant be removed and the code with RelayMutationTracker only be run when clientMutationID is in the payload?

I haven't looked at the functionality of RelayMutationTracker yet -- todo list for tomorrow -- so this might answer itself.

  1. Finally, I looked at RxJS to try and familiarize myself with the lingo. Here is rough pseudo-code for Relay#subscribe:
  subscribe(subscription, callbacks) {
    // the RelaySubscriptionObserver class does two things:
    // - add an onNext for calling writeRelayUpdatePayload
    // - enforce all the observer rules / laws / etc
    const observer = new RelaySubscriptionObserver(subscription, store /* or wuteva */, callbacks);
    const request = new RelaySubscriptionRequest(subscription, observer);

    // coerce the return value from RelayNetworkLayer#subscribe into a disposable e.g. Thing#dispose()
    const disposable = createDisposable(RelayNetworkLayer.sendSubscription(request));
    observer.setDisposable(disposable);
    return observer.getDisposable();
  }

code for RelayNetworkLayer#sendSubscription would return a function that performs unsubscribe / dispose:

  sendSubscription(request) {
    const id = 1; // placeholder ...

    const handler = response => {
      if (response.id === id) {
        if (response.data) {
          request.onNext(response.data);
        } else if (response.error) {
          request.onError(response.error);
        }
      }
    });

    socket.on(`graphql:subscription:${id}`, handler);

    // subscribe
    socket.emit('graphql:subscribe', {
      id,
      query: request.getQueryString(),
      variables: request.getVariables()
    });

    return () => {
      // unsubscribe
      socket.off('graphql:subscription', handler);
      socket.emit('graphql:unsubscribe', {id});
    };
  }

Seem reasonable?

Thanks!

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 15, 2016

I have an initial implementation of subscriptions and have questions / request of feedback from the Relay team if possible.

  1. Relay.Subscription / Relay.Mutation code duplication:

Right now I just duplicated code from Mutation to Subscription. This is mostly ok as its just a skeleton, but the function _resolveProps seems like logic that should be shared / not duplicated. Do you think this should be handled via extracting _resolveProps or making a base class or something? I know this is like a preference question, I just want to try and match accepted practices in Relay.

  1. clientSubscriptionId

I'm not sure why this is required.

  1. Query Building with MutationConfigs

With mutations the query is built from configs + fat query. With subscriptions it is provided. That said, the configs need to augment the query. For example, given a RANGE_ADD:

subscription {
  addTodoSubscribe(input: $input) {
    todoEdge {
      node { text complete }
    }
  }
}

The todoEdge field (edgeName) in the above query needs __typename and cursor added. They are added during the edge field creation for mutations. My assumption is that subscriptions should modify the provided query to make sure all required fields have been added. That would result in:

subscription {
  addTodoSubscribe(input: $input) {
    clientSubscriptionId
    todoEdge {
      __typename
      cursor
      node { text complete }
    }
  }
}

The logic I'm going with is:

  • clientSubscriptionId is added to everything
  • RANGE_ADD : add __typename to all edgeName fields (cursor is handled by Range.QL)
  • RANGE_DELETE / NODE_DELETE : add deletedIDFieldName to the call.

Is this reasonable?

  1. sanitizeRangeBehaviors

Should this be run against subscriptions as well? If so, would you suggest moving it from RelayMutationQuery into a different namespace as an export?

  1. breaking the PR up

A lot of this can be smaller PR's. I'm happy to break things up!


Thanks. I haven't really done much OSS work so I'm not really sure the workflow. I feel a bit blind ... just do stuff and submit a PR and see what happens? ;p

eyston commented Jan 15, 2016

I have an initial implementation of subscriptions and have questions / request of feedback from the Relay team if possible.

  1. Relay.Subscription / Relay.Mutation code duplication:

Right now I just duplicated code from Mutation to Subscription. This is mostly ok as its just a skeleton, but the function _resolveProps seems like logic that should be shared / not duplicated. Do you think this should be handled via extracting _resolveProps or making a base class or something? I know this is like a preference question, I just want to try and match accepted practices in Relay.

  1. clientSubscriptionId

I'm not sure why this is required.

  1. Query Building with MutationConfigs

With mutations the query is built from configs + fat query. With subscriptions it is provided. That said, the configs need to augment the query. For example, given a RANGE_ADD:

subscription {
  addTodoSubscribe(input: $input) {
    todoEdge {
      node { text complete }
    }
  }
}

The todoEdge field (edgeName) in the above query needs __typename and cursor added. They are added during the edge field creation for mutations. My assumption is that subscriptions should modify the provided query to make sure all required fields have been added. That would result in:

subscription {
  addTodoSubscribe(input: $input) {
    clientSubscriptionId
    todoEdge {
      __typename
      cursor
      node { text complete }
    }
  }
}

The logic I'm going with is:

  • clientSubscriptionId is added to everything
  • RANGE_ADD : add __typename to all edgeName fields (cursor is handled by Range.QL)
  • RANGE_DELETE / NODE_DELETE : add deletedIDFieldName to the call.

Is this reasonable?

  1. sanitizeRangeBehaviors

Should this be run against subscriptions as well? If so, would you suggest moving it from RelayMutationQuery into a different namespace as an export?

  1. breaking the PR up

A lot of this can be smaller PR's. I'm happy to break things up!


Thanks. I haven't really done much OSS work so I'm not really sure the workflow. I feel a bit blind ... just do stuff and submit a PR and see what happens? ;p

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 16, 2016

sorry for being chatty... but...

here is the commit with the work / comments in it: eyston@4405fa7

here is a stubbed implementation of a network layer: https://gist.github.com/eyston/ce723b38b1756cb5f81e

there are no tests atm, waiting on feedback on if this is sane or not ;p

thanks again~

eyston commented Jan 16, 2016

sorry for being chatty... but...

here is the commit with the work / comments in it: eyston@4405fa7

here is a stubbed implementation of a network layer: https://gist.github.com/eyston/ce723b38b1756cb5f81e

there are no tests atm, waiting on feedback on if this is sane or not ;p

thanks again~

@m64253

This comment has been minimized.

Show comment
Hide comment
@m64253

m64253 Jun 16, 2016

Last half of this talk brings up this topic http://youtu.be/ViXL0YQnioU

m64253 commented Jun 16, 2016

Last half of this talk brings up this topic http://youtu.be/ViXL0YQnioU

@papigers

This comment has been minimized.

Show comment
Hide comment
@papigers

papigers Jun 16, 2016

@m64253 Cool, @stream and @defer also seems awesome. Any eta for any of these to be released?
Is there any interim solution for implementing live updates in the meantime?

papigers commented Jun 16, 2016

@m64253 Cool, @stream and @defer also seems awesome. Any eta for any of these to be released?
Is there any interim solution for implementing live updates in the meantime?

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Jun 16, 2016

Contributor

Cool, @stream and @defer also seems awesome. Any eta for any of these to be released?

@papigers These are experimental features that we are still exploring. For an interim approach to real-time subscriptions, my comment in this thread.

Contributor

josephsavona commented Jun 16, 2016

Cool, @stream and @defer also seems awesome. Any eta for any of these to be released?

@papigers These are experimental features that we are still exploring. For an interim approach to real-time subscriptions, my comment in this thread.

@edvinerikson

This comment has been minimized.

Show comment
Hide comment
@edvinerikson

edvinerikson Jun 17, 2016

Contributor

I got real-time working by using the mutation api.
I plan on releasing the code (and example) when I have cleaned it up a little bit.

The current api allows you to have a similar api as mutations.

new AddTodoSubscription({ viewer: this.props.viewer });

class AddTodoSubscription extends RelaySubscriptions.Subscription {
  static fragments = {
    viewer: () => Relay.QL`
    fragment on User {
      id
      totalCount
    }`,
  };
  getSubscription() {
    return Relay.QL`subscription {
      addTodoSubscription {
        clientMutationId
        todoEdge {
          __typename
          node {
            __typename
            id
            text
            complete
          }
        }
        viewer {
          id
          totalCount
        }
      }
    }`;
  }
  getVariables() {
    return {};
  }
  getConfigs() {
    return [{
      type: 'RANGE_ADD',
      parentName: 'viewer',
      parentID: this.props.viewer.id,
      connectionName: 'todos',
      edgeName: 'todoEdge',
      rangeBehaviors: () => 'append',
    }];
  }
}

PoC

Contributor

edvinerikson commented Jun 17, 2016

I got real-time working by using the mutation api.
I plan on releasing the code (and example) when I have cleaned it up a little bit.

The current api allows you to have a similar api as mutations.

new AddTodoSubscription({ viewer: this.props.viewer });

class AddTodoSubscription extends RelaySubscriptions.Subscription {
  static fragments = {
    viewer: () => Relay.QL`
    fragment on User {
      id
      totalCount
    }`,
  };
  getSubscription() {
    return Relay.QL`subscription {
      addTodoSubscription {
        clientMutationId
        todoEdge {
          __typename
          node {
            __typename
            id
            text
            complete
          }
        }
        viewer {
          id
          totalCount
        }
      }
    }`;
  }
  getVariables() {
    return {};
  }
  getConfigs() {
    return [{
      type: 'RANGE_ADD',
      parentName: 'viewer',
      parentID: this.props.viewer.id,
      connectionName: 'todos',
      edgeName: 'todoEdge',
      rangeBehaviors: () => 'append',
    }];
  }
}

PoC

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 17, 2016

Contributor

Is this an official Facebook thing? 😄

Contributor

taion commented Jun 17, 2016

Is this an official Facebook thing? 😄

@edvinerikson

This comment has been minimized.

Show comment
Hide comment
@edvinerikson

edvinerikson Jun 17, 2016

Contributor

@taion Nope, I'm just playing in the wild 😄

Edit
I'm not an FB employee

Contributor

edvinerikson commented Jun 17, 2016

@taion Nope, I'm just playing in the wild 😄

Edit
I'm not an FB employee

@nodkz

This comment has been minimized.

Show comment
Hide comment
@nodkz

nodkz Jun 17, 2016

Contributor

@edvinerikson incredible thing!

I planned to implement it after finishing graphql-compose as middleware on the server side for schema. And on the client side via react-relay-network-layer as middleware for websockets for next major release.

So I am looking forward to your code release. You'll save me a lot of time by your working solution. You are my idol on this week!

Contributor

nodkz commented Jun 17, 2016

@edvinerikson incredible thing!

I planned to implement it after finishing graphql-compose as middleware on the server side for schema. And on the client side via react-relay-network-layer as middleware for websockets for next major release.

So I am looking forward to your code release. You'll save me a lot of time by your working solution. You are my idol on this week!

@tjmehta

This comment has been minimized.

Show comment
Hide comment
@tjmehta

tjmehta Jun 17, 2016

@edvinerikson , I'd love to see how you've implemented what you have, even before a release. I'm relatively new to relay, and I am not familiar with the source yet. I am about to attempt to implement subscriptions in relay myself. Seeing your modifications would help me see the effected parts of relay, and allow me to make the same or similar changes to get a temporary solution into my project much much faster.

Thanks and let me know!

tjmehta commented Jun 17, 2016

@edvinerikson , I'd love to see how you've implemented what you have, even before a release. I'm relatively new to relay, and I am not familiar with the source yet. I am about to attempt to implement subscriptions in relay myself. Seeing your modifications would help me see the effected parts of relay, and allow me to make the same or similar changes to get a temporary solution into my project much much faster.

Thanks and let me know!

@edvinerikson

This comment has been minimized.

Show comment
Hide comment
@edvinerikson

edvinerikson Jun 17, 2016

Contributor

An initial version is available at edvinerikson/relay-subscriptions. Feel free to do whatever you want with it (PRs very welcome 😄 ). I am happy to answer any questions you have about it in the new repo.

Contributor

edvinerikson commented Jun 17, 2016

An initial version is available at edvinerikson/relay-subscriptions. Feel free to do whatever you want with it (PRs very welcome 😄 ). I am happy to answer any questions you have about it in the new repo.

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Aug 16, 2016

Contributor

Given that #1298 is closed and that relay-subscriptions is a library, is there anything additional to track here?

Contributor

taion commented Aug 16, 2016

Given that #1298 is closed and that relay-subscriptions is a library, is there anything additional to track here?

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Aug 17, 2016

Contributor

@taion We get a fair number of questions about this, so I think leaving this open for now makes sense. I've updated the description though, to make it clear that we are not actively pursuing a full subscriptions API within the core.

Contributor

josephsavona commented Aug 17, 2016

@taion We get a fair number of questions about this, so I think leaving this open for now makes sense. I've updated the description though, to make it clear that we are not actively pursuing a full subscriptions API within the core.

@nodkz

This comment has been minimized.

Show comment
Hide comment
@nodkz

nodkz Aug 17, 2016

Contributor

@josephsavona if not subscriptions, then I think you are almost ready to release 1.0.0 with new features, better performance and new mutation api.

Most of all I am waiting new mutation api, cause all other things are quite comfortable. For me Relay is the best store keeper, than ReduxAppolo things.

Contributor

nodkz commented Aug 17, 2016

@josephsavona if not subscriptions, then I think you are almost ready to release 1.0.0 with new features, better performance and new mutation api.

Most of all I am waiting new mutation api, cause all other things are quite comfortable. For me Relay is the best store keeper, than ReduxAppolo things.

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Aug 19, 2016

Contributor

@josephsavona

Looking through #1298 and discussing with @edvinerikson – would you be okay with merging the scaffolding for subscription support in #1298?

I mean specifically https://github.com/facebook/relay/pull/1298/files#diff-320b6df8cf530a681d201c75772401eaR163, https://github.com/facebook/relay/pull/1298/files#diff-3a2c3b1ea174f413b5118b1aac4ecc2eR115, and an skeletal implementation of RelayEnvironment#subscribe that just throws.

This would allow actually implementing subscriptions in user-space, but still maintaining first-class API support.

Right now, with relay-subscriptions, an additional HoC is required to inject subscription support into components, which feels unnecessary given that there already is a Relay environment and a Relay container.

Contributor

taion commented Aug 19, 2016

@josephsavona

Looking through #1298 and discussing with @edvinerikson – would you be okay with merging the scaffolding for subscription support in #1298?

I mean specifically https://github.com/facebook/relay/pull/1298/files#diff-320b6df8cf530a681d201c75772401eaR163, https://github.com/facebook/relay/pull/1298/files#diff-3a2c3b1ea174f413b5118b1aac4ecc2eR115, and an skeletal implementation of RelayEnvironment#subscribe that just throws.

This would allow actually implementing subscriptions in user-space, but still maintaining first-class API support.

Right now, with relay-subscriptions, an additional HoC is required to inject subscription support into components, which feels unnecessary given that there already is a Relay environment and a Relay container.

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Aug 19, 2016

Contributor

@taion Is the main reason for adding those to avoid an extra HOC for those components that have subscriptions? If that's the case, this can probably be handled purely in user space. You could, for example, create a RelaySubscriptionContainer.create(Component, subscriptions, spec) function that delegates to Relay.createContainer (so that users only have to write one wrapper function instead of two). The extra HOC should be trivial in practice, given that relatively few containers would have subscriptions.

Please let me know if I'm overlooking something though!

Contributor

josephsavona commented Aug 19, 2016

@taion Is the main reason for adding those to avoid an extra HOC for those components that have subscriptions? If that's the case, this can probably be handled purely in user space. You could, for example, create a RelaySubscriptionContainer.create(Component, subscriptions, spec) function that delegates to Relay.createContainer (so that users only have to write one wrapper function instead of two). The extra HOC should be trivial in practice, given that relatively few containers would have subscriptions.

Please let me know if I'm overlooking something though!

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Aug 21, 2016

Contributor

That works.

One more thing – it looks like babel-relay-plugin has a special carve-out for clientSubscriptionId on subscription payloads.

It seems easier all around to keep track of identifying subscriptions in the network layer. There's no bookkeeping like with mutation queues I'm aware of that requires clientSubscriptionId at the library level.

In fact, #1298 doesn't implement clientSubscriptionId.

Am I missing any benefit that in fact does obtain from using clientSubscriptionId?

Contributor

taion commented Aug 21, 2016

That works.

One more thing – it looks like babel-relay-plugin has a special carve-out for clientSubscriptionId on subscription payloads.

It seems easier all around to keep track of identifying subscriptions in the network layer. There's no bookkeeping like with mutation queues I'm aware of that requires clientSubscriptionId at the library level.

In fact, #1298 doesn't implement clientSubscriptionId.

Am I missing any benefit that in fact does obtain from using clientSubscriptionId?

@tjmehta

This comment has been minimized.

Show comment
Hide comment
@tjmehta

tjmehta Aug 24, 2016

I did not include clientSubscriptionId, bc i ended up just handling it in the network layer ( like josephsavona pointed out it could be done, earlier in this thread: #541 (comment)). In my full implementation, it was only used in the network layer to the network layer to correlate requests and responses for primus ( this may not be necessary for other transports that handle this correlation for you, like socket.io).

tjmehta commented Aug 24, 2016

I did not include clientSubscriptionId, bc i ended up just handling it in the network layer ( like josephsavona pointed out it could be done, earlier in this thread: #541 (comment)). In my full implementation, it was only used in the network layer to the network layer to correlate requests and responses for primus ( this may not be necessary for other transports that handle this correlation for you, like socket.io).

@jg123

This comment has been minimized.

Show comment
Hide comment
@jg123

jg123 Nov 18, 2016

I am toying with the idea of transforming our existing mutation payloads into subscription payloads. That way, only one payload would need to be maintained instead of two. I am currently able to take a mutation class and either extract a RelayMutationQuery object or the actual graphql string. Neither of these representations get me to the point where I can transform it into the object like that which is created when passing a template string into the Relay.QL function which can be used to create a subscription object from the RelayQuery.Subscription.create method. Any ideas? I can paste code examples if you need more context for what I am attempting.

jg123 commented Nov 18, 2016

I am toying with the idea of transforming our existing mutation payloads into subscription payloads. That way, only one payload would need to be maintained instead of two. I am currently able to take a mutation class and either extract a RelayMutationQuery object or the actual graphql string. Neither of these representations get me to the point where I can transform it into the object like that which is created when passing a template string into the Relay.QL function which can be used to create a subscription object from the RelayQuery.Subscription.create method. Any ideas? I can paste code examples if you need more context for what I am attempting.

@tjmehta

This comment has been minimized.

Show comment
Hide comment
@tjmehta

tjmehta commented Nov 18, 2016

@jg123 quick note for now check out http://npmrepo.com/relay-subscriptions or http://npmrepo.com/primus-graphql

I am using primus-graphql in prod for https://codeshare.io

@jg123

This comment has been minimized.

Show comment
Hide comment
@jg123

jg123 Nov 18, 2016

Thanks @tjmehta. I have been borrowing heavily from the relay-subscriptions project, and primus looks nice and concise. What I really want to do is use my current mutation payloads as subscription payloads, so I only need to maintain the mutations.

jg123 commented Nov 18, 2016

Thanks @tjmehta. I have been borrowing heavily from the relay-subscriptions project, and primus looks nice and concise. What I really want to do is use my current mutation payloads as subscription payloads, so I only need to maintain the mutations.

@edvinerikson

This comment has been minimized.

Show comment
Hide comment
@edvinerikson

edvinerikson Nov 18, 2016

Contributor

If you use fragments you can share that fragment between the mutation and
subscription. ${TodoMutation.getFragment('todo')}

On Fri, 18 Nov 2016 at 23:09, Josh Geller notifications@github.com wrote:

Thanks @tjmehta https://github.com/tjmehta. I have been borrowing
heavily from the relay-subscriptions project, and primus looks nice and
concise. What I really want to do is use my current mutation payloads as
subscription payloads, so I only need to maintain the mutations.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#541 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADtdjD3s2CBgfJlhrvvrdtIfI3g5xKkAks5q_iItgaJpZM4GaY63
.

Contributor

edvinerikson commented Nov 18, 2016

If you use fragments you can share that fragment between the mutation and
subscription. ${TodoMutation.getFragment('todo')}

On Fri, 18 Nov 2016 at 23:09, Josh Geller notifications@github.com wrote:

Thanks @tjmehta https://github.com/tjmehta. I have been borrowing
heavily from the relay-subscriptions project, and primus looks nice and
concise. What I really want to do is use my current mutation payloads as
subscription payloads, so I only need to maintain the mutations.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#541 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADtdjD3s2CBgfJlhrvvrdtIfI3g5xKkAks5q_iItgaJpZM4GaY63
.

@jg123

This comment has been minimized.

Show comment
Hide comment
@jg123

jg123 Nov 19, 2016

Thanks @edvinerikson. Maybe a mutation doesn't translate to a subscription. The fat query intersection is dynamic based on previous data retrieved and doesn't make sense in the context of a more static subscription payload.

jg123 commented Nov 19, 2016

Thanks @edvinerikson. Maybe a mutation doesn't translate to a subscription. The fat query intersection is dynamic based on previous data retrieved and doesn't make sense in the context of a more static subscription payload.

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Sep 11, 2017

Contributor

I'm closing this conversation thread.

Great news is that Relay Modern supports both GraphQL Subscriptions and "live" queries via an Observable based network layer!

Contributor

leebyron commented Sep 11, 2017

I'm closing this conversation thread.

Great news is that Relay Modern supports both GraphQL Subscriptions and "live" queries via an Observable based network layer!

@leebyron leebyron closed this Sep 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment