search

feathersjs-ecosystem / feathers-sync

Synchronize service events between Feathers application instances
MIT License
222 stars 41 forks source link

Is serialization of the complete context required in event ? #87

Closed claustres closed 4 years ago

claustres commented 6 years ago

Starting from v1.0 not only the event data, but also the call context, is serialized as part of the event (at least for MongoDB, see https://github.com/feathersjs-ecosystem/feathers-sync/blob/master/lib/adapters/mongodb.js#L29). This already raised the following issue https://github.com/feathersjs-ecosystem/feathers-sync/issues/83, linked to the fact the context can be a really complex object structure not really suited for serialization.

The "stringification" used to tackle this issue lead to data that are hard to read in the DB from a debug perspective. Moreover, only client data are really safe to be serialized IMHO. I wonder why the context serialization is actually required because from the client perspective only the data is sent back. I think removing it should avoid a lot of potential issues like performance overhead, failure to serialize cyclic JSON structures, etc.

daffl commented 6 years ago

People were expecting the hook context to be sent with service events. For example, context.params.user is pretty important for proper event dispatching.

claustres commented 6 years ago

For sure having event raised on the distant server with the same context as on the source server is great. However params is often used as a catch-all to store any information required by a service, e.g. it might hold some references to other services, internal objects, etc. making it not suitable for serialization IMHO (otherwise it should be clearly stated in the doc). Because params is not sent back to client I guess people usually do not clean it up after the service call, as I do I must confess 😄

I wonder if at least it will be interesting to be able to set some "filtering" functions allowing to define what information is useful to be serialized. By default serializing only the user could be a relevant approach since it is the sole information really required by Feathers' core.

Otherwise do you recommend to create after hooks to clean the params object before serialization ? It seems to me a little painful but should work since events are emitted after the hooks have been run if I am not wrong.

Thanks for your answer.

daffl commented 4 years ago

I think this can now also be handled in v1.2.0 and later by using a custom serializer/deserializer.