feat(src/classes/job.ts): add custom serializer/deserializer for job data

[godinja]

ref #14

[manast]

Thank you for the PR. The issue with serializers is that if we allow a completely custom serializer, then all the dashboard tools will break, as they will not be able to show any of the job data. Therefore the functionality must be implemented in such a way that we offer a set of predefined serializers, for example json, protobuf, messagepack, etc, so that when getting a job we also can apply the correct deserializer.

[godinja]

The issue with serializers is that if we allow a completely custom serializer, then all the dashboard tools will break, as they will not be able to show any of the job data.

The dashboards are a third party offering and aren't necessarily required to use BullMQ, right? Couldn't we just inform the user that using custom serializers and deserializers may break third party integrations? Considering that this is an opt-in feature, I feel like it's an appropriate trade off.

Therefore the functionality must be implemented in such a way that we offer a set of predefined serializers, for example json, protobuf, messagepack, etc, so that when getting a job we also can apply the correct deserializer.

What about restricting the type definitions a bit more:

type JsonObject = { [key: string]: JsonValue };

type JsonValue =
  | null
  | boolean
  | number
  | string
  | JsonValue[]
  | JsonObject;

type SerializerFn = (data: any) => JsonValue; // stringify return value with JSON.stringify in Job.asJSON()
type DeserializeFn = (data: string) => JsonValue;

That way the data is guaranteed to be parsable by JSON.parse. In our use case we plan on using superjson which serializes the data into a JSON-compatible structure.

[manast]

The dashboards are a third party offering and aren't necessarily required to use BullMQ, right? Couldn't we just inform the user that using custom serializers and deserializers may break third party integrations? Considering that this is an opt-in feature, I feel like it's an appropriate trade off.

Yes, they are third-party but a very important piece in BullMQ's ecosystem, as they are super helpful for debugging and managing production systems.

That way the data is guaranteed to be parsable by JSON.parse. In our use case, we plan on using superjson which serializes the data into a JSON-compatible structure.

I feel that would be still too specific as that would limit the possibility of having other possibly more efficient serialization methods in the future. I still think the best way is to define a set of fixed serialization/deserialization methods, but I haven't figured out yet what would be the best way to do this so that we do not need to add a bunch of extra dependencies to BullMQ, but instead optional add-ons that can be added only if required.

[godinja]

That way the data is guaranteed to be parsable by JSON.parse. In our use case, we plan on using superjson which serializes the data into a JSON-compatible structure.

I feel that would be still too specific as that would limit the possibility of having other possibly more efficient serialization methods in the future. I still think the best way is to define a set of fixed serialization/deserialization methods, but I haven't figured out yet what would be the best way to do this so that we do not need to add a bunch of extra dependencies to BullMQ, but instead optional add-ons that can be added only if required.

Ah I see, the long term vision for this is to be extensible for other data formats, not just JSON.

Considering that #14 has been open for almost 5 years, how would you feel about releasing something like this PR to allow for custom JSON serializers?

[manast]

Considering that #14 has been open for almost 5 years, how would you feel about releasing something like this PR to allow for custom JSON serializers?

I created that issue because I think it is a useful feature, however, I do not think it has gained so much interest over the years, otherwise it would have been up-prioritized. But this PR is enabling something that potentially can break current UIs, so I am not so positive to merge it as is. If we can figure out a better way, then yes I am open to it.

[godinja]

But this PR is enabling something that potentially can break current UIs, so I am not so positive to merge it as is. If we can figure out a better way, then yes I am open to it.

I've made some updates and included a pattern doc (with a note on how this may effect third-party integrations). With the new updates, I don't really see how this could break UIs.

[HsinHeng]

@godinja Hi, Thanks a lot for your contributes.

In my opinion, I think that do twice serializations of serializer(rawData) and JSON.stringify is not good idea for node.js event loop.
Can we just design once serialization/deserialization for data?

maybe like

{
   data: serializer(rawData),
   ...
}