localDocument.$.subscribe invoked twice for an atomic insert

[de-robat]

Hello, first of all kudos for this generally well thought of and nicely running software. This Issue is rather to be considered a question before opening a potential PR.

We recently run into a problem when upserting localDocuments. We have a line that reads as follows:

document.$.subscribe((doc: MyState) => {
        console.log(doc)
       ...
      })

And this one gets invoked twice for a single upsert. I could pinpoint the issue to this part of the code:

https://github.com/pubkey/rxdb/blob/master/src/plugins/local-documents.ts#L195
which reads like this:

_saveData(this: any, newData: any) {
        const oldData = this._dataSync$.getValue();
        newData = clone(newData);
        newData._id = LOCAL_PREFIX + this.id;

        const startTime = now();
        return this.parentPouch.put(newData)
            .then((res: any) => {
                const endTime = now();
                newData._rev = res.rev;
>>>         this._dataSync$.next(newData);

                const changeEvent = new RxChangeEvent(
                    'UPDATE',
                    this.id,
                    clone(this._data),
                    isRxDatabase(this.parent) ? this.parent.token : this.parent.database.token,
                    isRxCollection(this.parent) ? this.parent.name : null,
                    true,
                    startTime,
                    endTime,
                    oldData,
                    this
                );
>>>         this.$emit(changeEvent);
            });
    },

The two marked lines both invoke the observables listener. I do not understand if this is intended. To add some context, this basically leads for a react application of ours to invoke unnecessary (virtual) rendering cycles which we would like to avoid.

Thanks in advance!

[pubkey]

@de-robat Good investigation. I think you have found a bug and we should change that. As you have pointed out, doing this.$emit(.. will trigger a new invocation downwards.

[de-robat]

Ty for the quick reply, i'll prepare a PR and verify the integrity by running the tests.

[de-robat]

Ok, unfortunaltey it not just a matter of removing the line. The tests will fail at:

1) local-documents.test.js
       .upsertLocal()
         positive
           should update when exists:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

'bar' !== 'bar2'

      + expected - actual

      -bar
      +bar2

So i have to postpone a PR as i have no clue about the architecture around it. :/

[pubkey]

Can you make a PR with a test that reproduces the problem you have? I will then continue with the fix.

[styx0r]

Hi folks, I created a PR with the corresponding test.

I observed also that using .$.subscribe on a localDocument
the subscription is always fired initially. I was wondering whether this is intended.
If so, the test I put into the PR should be adapted by exchanging

assert.strictEqual(invocations, 1);

to

assert.strictEqual(invocations, 2);

Thank you very much for your efforts!

[de-robat]

Ty @christbald and @pubkey , let me know if I can be of further assistance.

[pubkey]

I fixed this now. The handling of updateData is now similar to a non-local document.
@christbald yes the document.$ should behave like a BehaviorSubject. So it emits the current value on subscription and every ongoing value on changes.