-
Notifications
You must be signed in to change notification settings - Fork 452
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
✨ Allow to preserve previous snapshot version that will be available in commit middleware #667
Conversation
…in commit middleware Occasionally, it is necessary to reference the previous version of a snapshot to execute auxiliary operations within the commit middleware, for example: Let's imagine we have this doc that describes user: ```typescript { id: '123', emailConfirmed: false; } ``` Now we would want to send notification to the admin whenever `emailConfirmed` changed from `false` to `true` At the moment there is no simple way to check in the middlewares if something is changing, apart form trying to apply the ops manually in the `apply` middleware. Something like ```typescript backend.use('apply', function(request, next) { const changedSnapshot = applyOps(clone(request.snapshot), request.op); if( request.snapshot.data.emailConfirmed === false && request.snapshot.data.emailConfirmed === true ) { notifyAdmin(); } }) ``` This approach has few drawbacks: 1. We need to apply ops twice once in this middleware and once in the sharedb itself. 2. It is quite difficult for app to make considerations and mimick the flow of `$fixOps` Solution --- Let's make it possible to opt-in for storing the pre-apply snapshot version in request by setting a `preservePreapplySnapshot` flag to `true` in any middleware that happens before apply. This will allow us to: 1. Have the old and new version of snapshot available in the `commit` middleware 2. We do not have much memory overhead as this is opt-in, so if user never needs it the snapshot would never be cloned. 3. We only need to apply ops once 4. The apply mechanism is using shareDb power house.
* (snapshot without the ops applied) and the new snapshot (with the ops applied) | ||
*/ | ||
if (request.preservePreapplySnapshot) { | ||
request.preapplySnapshot = util.clone(request.snapshot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I see this written down, is this any more advantageous than consumers just doing this themselves in middleware directly?
With this approach:
backend.use('apply', (request, next) => {
request.preservePreapplySnapshot = true;
});
Or we can just do this with no changes to ShareDB at all:
backend.use('apply', (request, next) => {
request.preapplySnapshot = clone(request.snapshot);
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah actaully you are right, but there is an argument to be made that it should be part of sharedb itself for simplicity sake, but I am fine to just close this PR and just add some example of this use case in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided to just add the documentation about this use case
As per this discussion: - #667 (comment)
As per this discussion: - #667 (comment)
As per this discussion: - #667 (comment)
As per this discussion: - #667 (comment)
Occasionally, it is necessary to reference the previous version of a snapshot to execute auxiliary operations within the commit middleware, for example:
Let's imagine we have this doc that describes user:
Now we would want to send notification to the admin whenever
emailConfirmed
changed fromfalse
totrue
At the moment there is no simple way to check in the middlewares if something is changing, apart form trying to apply the ops manually in the
apply
middleware. Something likeThis approach has few drawbacks:
$fixOps
Solution
Let's make it possible to opt-in for storing the pre-apply snapshot version in request by setting a
preservePreapplySnapshot
flag totrue
in any middleware that happens before apply. This will allow us to:commit
middleware