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

on 'op' callback firing multiple times per each submitOp #147

Open
mdelbuono opened this issue Apr 25, 2017 · 8 comments
Open

on 'op' callback firing multiple times per each submitOp #147

mdelbuono opened this issue Apr 25, 2017 · 8 comments

Comments

@mdelbuono
Copy link

Hi I have the following code:

shareDoc.on('op', (ops, source) => {
    for (let op of ops) {
        this._dispatchOperation(op);
    }
});

The _dispatchOperation method gets called 4 times for each submitOp with the same ops object and source=true.

Is this by design or is it a bug?

In my case it is not what I want, therefore I modified the code as follows.

this._lastOps = null;
shareDoc.on('op', (ops, source) => {
    if (lodash.isEqual(ops, this._lastOps)) return;
    this._lastOps = ops;
    for (let op of ops) {
        this._dispatchOperation(op);
    }
});

Is this ok or the three op notifications that I am ignoring may be important?

@nickasd
Copy link

nickasd commented May 16, 2017

I'm experiencing the same issue but I'm not sure that they are related. In my case I noticed that the callback is fired for each doc.on('op', ...) call, even when calling doc.destroy() in between. In my opinion, either doc.destroy() should remove all registered callbacks, or there should be some method like doc.off('op').

@nickasd
Copy link

nickasd commented Jun 26, 2017

Thanks to #161 I discovered the method doc.removeListener('op', callback) which seems to do exactly what doc.destroy() should be doing (unregistering all observers). This should be mentioned in the README file.

@curran
Copy link
Contributor

curran commented Jun 26, 2017

I encountered a similar issue and submitted a PR adding this documentation to the README 9 days ago #160 It has not yet been reviewed by @nateps or @rkstedman , who seem to be the only ones active on this project with permissions to merge pull requests.

@nickasd
Copy link

nickasd commented Jun 26, 2017

Thanks a lot, didn't see that.

@curran
Copy link
Contributor

curran commented Jun 26, 2017

Totally understandable! I wish I could do more to push these PRs through in a timely manner. Maybe you could help me lobby for attention from the maintainers ;)

@nickasd
Copy link

nickasd commented Jun 26, 2017

If there's something I can do just tell me ... I'm quite new to GitHub.

@curran
Copy link
Contributor

curran commented Jul 4, 2017

@nickasd Maybe make some noise over at #163? I feel like ShareDB is really awesome, but is suffering from absentee maintainers.

@ninjapapa
Copy link

I actually have to wrap doc.removeListener in doc.whenNothingPending to solve the problem.

In my code, it may happen that just after I call subscribe and fetch, I need to undo it right away. Since the attaching listener call, doc.on is inside the fetch's callback, so it can actually happen after the doc.removeListener get called. In that case doc.removeListener will do nothing, and two listener functions will be attached after the second time doc.fetch get called.

The following solved my problem:

      this.doc.whenNothingPending((err) => {
        this.doc.removeListener('op', this.handleSharedbUpdateCode);
      })

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

No branches or pull requests

4 participants