Allow to pass a custom serializer and deserializer

[quentinus95]

Fixes #182.

Allows to pass a custom deserialize function, for instance to retrieve class instances.

[quentinus95]

@davidyaha do you think this PR can be merged? Thx.

[davidyaha]

Hey @quentinus95! Sorry for the delay.

One question, could you explain in one sentence the difference between reviver and deserializer in your implementation?

Thanks ahead!

[davidyaha]

@quentinus95 Please add tests for when serializers or deserializers throws an error

[quentinus95]

What would you expect from such a situation? Current behavior, if I'm not wrong, is that the error would be thrown directly to the subscriber / dispatcher. Do you feel this behavior is wrong?

[davidyaha]

For the deserializer the code just ignores the error and gives back the message unmarshalled:
https://github.com/davidyaha/graphql-redis-subscriptions/pull/199/files#diff-3b3bf6f5369f165d6124779b61832724R162-R166
I feel like that's a behaviour that we can change if such a request arises from the community.
I would much rather have a discussion about that in a dedicated issue rather than change the behaviour.
So for the moment, don't change the package code but regardless, we should have tests demonstrating the behaviour.

Thanks!

[quentinus95]

@davidyaha if I'm not wrong, a reviver would take a single scalar value and allow to turn it into something else (e.g. a timestamp number to a date instance). The main issue is when you want to create a class instance from a set of properties. The deserializer allows you to use, for instance, https://github.com/typestack/class-transformer to create real class instances from a JSON object.

It can be very convenient to dispatch an event represented by a class instance, and re-create it from the serialized object in the subscriber.

[davidyaha]

Yeah, I now realize the change in behavior.
Basically JSON.parse with or without a custom reviver is the default deserializer.
Then, your PR gives the user the option to change the JSON.parse all around.

I think the codebase should reflect that granularity. would you mind changing the code to something of that sort:

const deserializer = this.derializer || (msg) => JSON.parse(msg, this.reviver);
try {
  parsedMessage = deserializer(message);
}

Thanks ahead

[asabhaney]

hey @quentinus95 I've submitted a PR to address the requested tests, hope that helps.

[quentinus95]

@asabhaney many thanks, I've just added your commit to the PR.

[quentinus95]

@davidyaha non passing tests are unrelated to the changes.

[asabhaney]

@davidyaha any chance of having this PR merged in soon? If there's anything I can do, I'm happy to help move this along. Thanks!

[davidyaha]

Hey, @asabhaney and @quentinus95!
Thanks for the PR and sorry for the delay.
It seems like the issues with the build are indeed unrelated so I'll merge that PR.

[davidyaha]

Release version 2.2.0. LMK if anything is wrong there.

[asabhaney]

Great, thanks so much @davidyaha @quentinus95!