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

[Feature] Support messages different than strings (messageBuffer) #182

Closed
nuragic opened this issue Sep 27, 2019 · 3 comments · Fixed by #199
Closed

[Feature] Support messages different than strings (messageBuffer) #182

nuragic opened this issue Sep 27, 2019 · 3 comments · Fixed by #199

Comments

@nuragic
Copy link

nuragic commented Sep 27, 2019

Hello! First of all thanks for your time and your work on making and maintaining this lib! 👏

We're trying to migrate from the basic 'graphql-subscriptions' package to this one.

We have a use case in which we don't just publish a single change, e.g. a plain object { ... }, but a bunch of them; so we publish a Set([ ... ]) of changes in one shot. The problem is that the code is doing this:

public async publish<T>(trigger: string, payload: T): Promise<void> {
    await this.redisPublisher.publish(trigger, JSON.stringify(payload));
  }

Here the Set is lost because of the assumption that everything would be a String. 😄

However, ioredis supports publishing Buffers as well:

redis.on("messageBuffer", function(channel, message) {
  // Both `channel` and `message` are buffers.
});

Would you be open to add this feature?

Many thanks! 🙏

@nuragic nuragic changed the title Support message type different than strings Support messages different than strings Sep 27, 2019
@nuragic
Copy link
Author

nuragic commented Sep 27, 2019

Oh, I've just realised that this feature request isn't really needed for this particular case, because if we convert the Set to Array before publishing, and viceversa on the Subscription resolve()... it still works. 😅

const changes = new Set();
changes.add({ whatever });
// …
pubSubInstance.publish('TOPIC', {
  customChangeSet: [...changes],
});

// then on the subscription resolver:
resolve: (payload, args, context, info) => {
  const changes = new Set(payload.customChangeSet);
  // …
}

Sorry for the noise! Let me know anyway if you would keep this open since it's still a missing feature if you want to publish e.g. binary data.

Thanks!

@nuragic nuragic changed the title Support messages different than strings [Feature] Support messages different than strings (messageBuffer) Sep 27, 2019
@davidyaha
Copy link
Owner

Hey @nuragic!

Thanks for the suggestion! I would love to review a PR if someone would like to have a go with it. Preferably, someone that will use this feature on production so they could give us good feedback regarding the results.

@quentinus95
Copy link
Contributor

I just published a pull request which may address this issue as well.
#199

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

Successfully merging a pull request may close this issue.

3 participants