Update hook's obj is not an instance of the mapped class

[wwoods]

Using Dexie's "updating" hook, I expected the obj member to be an instance of the mapped class, and as such called a method on it. I can understand why this is not the case, since the mods argument contains updates to the obj argument, which reflects the previous state of the object. However, the docs do not mention this, so it is an issue. I'd propose something along the lines of:

1. The documentation for the update hook should clearly specify that obj contains the old revision's data, but is not coerced to the mapped class.
2. The mods object contains updates that will be applied to obj, but haven't yet. That is, users must manually apply the mods object over obj if they want to see the updated version.
3. It should be noted somewhere that mods.[arrayMember] will never exist; rather, Dexie uses a custom name.index format for these types of modifications.
4. It might be wise, in light of point 3, to include a Dexie.merge utility function somewhere for the merge hook, that returns a new object of the mapped class that is obj with mods applied.

Thoughts?

[wwoods]

Note: modify also suffers from this... That case is more confusing though, as modify gets the whole object.

[dfahlander]

In the documentation of Table.mapToClass, this behavior has been documented for quite a while. The reason is performance.

If you need to get the updated version of obj from your update hook, you may use help methods Dexie.deepClone() to clone, Dexie.setByKeyPath() and Dexie.delByKeyPath() to apply given modifications to given object.

Pasting a code snipped from Dexie.Observable that does this :

      table.hook('updating').subscribe(function(mods, primKey, oldObj, trans) {
            // mods may contain property paths with undefined as value if the property
            // is being deleted. Since we cannot persist undefined we need to act
            // like those changes is setting the value to null instead.
            var modsWithoutUndefined = {};
            // As of current Dexie version (1.0.3) hook may be called even if it wouldnt really change.
            // Therefore we may do that kind of optimization here - to not add change entries if
            // there's nothing to change.
            var anythingChanged = false;
            var newObj = Dexie.deepClone(oldObj);
            for (var propPath in mods) {
                var mod = mods[propPath];
                if (typeof mod === 'undefined') {
                    Dexie.delByKeyPath(newObj, propPath);
                    modsWithoutUndefined[propPath] = null; // Null is as close we could come to deleting a property when not allowing undefined.
                    anythingChanged = true;
                } else {
                    var currentValue = Dexie.getByKeyPath(oldObj, propPath);
                    if (mod !== currentValue && JSON.stringify(mod) !== JSON.stringify(currentValue)) {
                        Dexie.setByKeyPath(newObj, propPath, mod);
                        modsWithoutUndefined[propPath] = mod;
                        anythingChanged = true;
                    }
                }
            }

           ... 

[wwoods]

I looked at this more, and the reason it wasn't immediately obvious is because the word "hook" doesn't show up in the mapToClass page (I searched for it and didn't see it) - you have a positive list, but no negative list. Might be good to add to the wiki.

[dfahlander]

Thanks for the input. It could definitely be important to mention the hooks there.

[wwoods]

I added a paragraph at the bottom of Remarks: https://github.com/dfahlander/Dexie.js/wiki/Table.mapToClass()#remarks

Take a look, let me know if it looks good, I'll close the issue if it does or change it if it doesn't.

Edit: Actually, I'm still a fan of providing Dexie.makeObj(table, obj[, mods]) to prevent a lot of boilerplate code / keep things simple for users. What do you think about that API? I'm not entirely happy with the name or need for the table argument...

[dfahlander]

Thanks for adding the notice in wiki!

Dexie.makeObj would be a nice low-level helper function. But in the plans for next major, new hook APIs will enable async bulk hooks instead of the current sync single-object hooks. Maybe these APIs will already provide mapped objects instead of as it works now. (I will make the new API backward compatible though through an adapter, so it's still safe to rely on the current API). I'm also trying to keep the size of dexie.min.js as low as possible and not introduce unnessecary things and once an API is introduced, it can be hard to remove it.

[wwoods]

I am curious what you mean about async bulk hooks, but I suppose I'll see when you release it :) Dexie has been well designed so far.

I definitely understand the reluctance, as APIs do need to be supported once they're released. Perhaps providing the function on a wiki page would be a better option (function makeObj(...)). Dexie.Observable is 50% of the way there anyway.