dexie / Dexie.js

A Minimalistic Wrapper for IndexedDB
https://dexie.org
Apache License 2.0
11.43k stars 641 forks source link

Dexie Sync: All databases getting synced on using Dexie.Syncable #454

Open puhazh opened 7 years ago

puhazh commented 7 years ago

I have two databases one is to be synced to a server (using WebSocket Sync) and the other is a local db for storing temporary changes and other things that I refer on the client. But both the databases are synced when I use Dexie Syncable.

Here is a sample code: ` var db1 = new Dexie("MySyncedDB");

db1.version(1).stores({
    friends: "$$oid,name,shoeSize",
    pets: "$$oid,name,kind"
});

db1.syncable.connect ("websocket", "wss://dexie-test-kspuhazh.c9users.io");
db1.syncable.on('statusChanged', function (newStatus, url) {
    console.log ("Sync Status changed: " + Dexie.Syncable.StatusTexts[newStatus]);
});

var db2 = new Dexie("MyLocalDB");

db2.version(1).stores({
    friends: "$$oid,name,shoeSize",
    pets: "$$oid,name,kind"
});`

Expected behaviour is only the MySyncedDB is expected to be synced, but I see both the databases getting synced.

Example on this link. You could see the sync related tables (_changes, _intercomm, etc.,) created on client side for both the databases.

How can I have an un-synced local database when I use Dexie Syncable?

puhazh commented 7 years ago

The changes are not synced, those collections are created by Dexie.Observable on all databases even though sync is not used on them. The local DB (db2) is not getting synced.

nponiros commented 7 years ago

As you noted Dexie.Observable adds those tables to all databases. There is currently no way to exclude a database from Dexie.Observable/Dexie.Syncable. I'm pretty sure that Dexie.Observable will be active for both databases but not Dexie.Syncable as it is bound to a specific database instance (the one you call db.connect() on). When Dexie.Observable is active and you have multiple open tabs which are running the same databases, then changes in one tab will propagate to the other one.

If you have stores that should not be observed by Dexie.Observable, you can use _ or $ in front of their names.

dfahlander commented 7 years ago

You can also use the option {addons: []} to Dexie constructor to inactivate addons for specific databases.

https://github.com/dfahlander/Dexie.js/wiki/Dexie#options

var plainDB = new Dexie ('dbname', {addons:[]}) ;
nponiros commented 7 years ago

Totally forgot about that option. Maybe Dexie should per default use {addons: []} and have the user explicitly request the addons he/she wants to use. This part also confused me when I started using Dexie and went into the code to see which addons are actually activated by default when addons is not defined explicitly.

dfahlander commented 7 years ago

Thought it might be nice to see addons as optional built-ins. But we lack a way to include then without applying them by default.

import dexieSyncableAddon from 'dexie-syncable/addon';

In contrast to just

import 'dexie-syncable';

My suggestion would be that the first one includes the addon and pushes it to Dexie.addons while the other just returns the addon without activating it.

Sync-client should use the second one if so since it extends Dexie and doesn't want to change the behavior for users including Dexie side by side with Sync-client.

In essence, addons (unlike plugins) would be seen as optional native behavior. That would justify the side effect.

But I am open for a discussion about this. I don't want people to get confused. I might be on the wrong path so please enlighten me if so :)

nponiros commented 7 years ago

Generally speaking I'm not a big fan of side effects. That being said, I do see the advantage of autoincluding addons implicitly by just importing the addon. I think what confused me was that there are two ways to define addons. Implicitly and globally by importing the addon and explicitly by using the addons array which is bound to a instance of Dexie.

I think having both ways to include an addon makes it unnecessarily complex/confusing. A further issue would be that an addon developer might not allow the implicit/global addon inclusion which would make the addons interface inconsistent and more complex for Dexie users.

I personally like passing addons as functions and registering explicitly and not implicitly. And in Dexie's case the addons would always be instance bound. But I'm also not against having two ES imports, one for globally adding an addon by automatically pushing to Dexie.addons and one for getting a function to use for the addons of an instance (or to be used by the user to push to Dexie.addons). An extra import for the global addons would make things more explicitly.

dfahlander commented 7 years ago

@nponiros I see. And I agree with you. The focus then is the documentation. There must be a clear documentation on how to include the addons. Current docs sais import 'dexie-syncable' to activate it. A clearer behaviour (and API change) would be something like this:

import Dexie from 'dexie';
import dexieObservable from 'dexie-observable';
import dexieSyncable from 'dexie-syncable';

const db = new Dexie ('dbname', {addons: [dexieObservable, dexieSyncable]});

To apply an addon by default on future Dexie instances:

Dexie.addons.push(dexieObservable);
Dexie.addons.push(dexieSyncable);

const db = new Dexie('dbname'); // Will get the addons without explicit addons option.

A shortcut for calling Dexie.addons.push() is to:

import Dexie from 'dexie';
import 'dexie-observable/apply-global';
import 'dexie-syncable/apply-global';

const db = new Dexie('dbname');

Thoughts?

nponiros commented 7 years ago

@dfahlander I like it. I think this is the way to go. We should contact other addon maintainers to see if they are willing to implement this change so we can have a consistent addons interface.

Dexie-relationships can not be applied globally from what I can tell and dexie-mongoify can only be applied globally.

@ignasbernotas @YurySolovyov any thoughts on the comment above regarding addon inclusion?

YurySolovyov commented 7 years ago

I like the idea, but won't be able to update mongoify right away

nponiros commented 7 years ago

@YurySolovyov if we go this way I can create a PR for you which you can release when dexie is released with the change.

Edit: dexie won't need changes, just the addons.

YurySolovyov commented 7 years ago

I'll take a look at PR if you submit one.

iberflow commented 7 years ago

I definitely agree, I'm more of a fan of explicitly defining things and consistency between addons is a must. It's a go from me. Let me know when you'd need this:)

dfahlander commented 7 years ago

Think we will need to overlook the d.ts files as they now act as if the api is extended when including the addon

nponiros commented 7 years ago

Hmm maybe the addons could return an object and dexie could extend itself with that object. In the d.ts we could just define the addon function and the returned object. I'm not really sure if what I'm suggesting will work but it's an idea. I think typescript would work even without changes but autocompletion will probably offer methods which might not exist.I believe that this is a general issue when objects are extended..

The better solution would probably be to write the addons (at least the ones with a d.ts) in typescript and remove the declarations.