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

'modifications' object in 'updating' hook can be bizarre #248

Closed
cherrydev opened this issue May 10, 2016 · 4 comments
Closed

'modifications' object in 'updating' hook can be bizarre #248

cherrydev opened this issue May 10, 2016 · 4 comments

Comments

@cherrydev
Copy link

So, I'm using some hooks to integrate moment.js support into Dexie, so I can have objects that use moment objects in code and persist to Date objects in IndexDB. My 'creating' and 'reading' hook work as expected, but I get a very bizarre issue in the 'updating' hook. My test entities just have an 'id, name, date' structure, but when I attempt to use put() to update an entity that has a new date, (where the date property is a moment object), the modification object passed into the hook does not contain a date property, as expected, but instead contains 7 properties, with the keys:

  • "date._d"
  • "date._f"
  • "date._i"
  • "date._isAMomentObject"
  • "date._isUTC"
  • "date._locale"
  • "date._pf"

Yes, those are 7 properties that have dot in the name of them. Those 7 properties correspond to the 7 properties that a moment object has inside of it.

I'm using Dexie 1.3.6 on Chrome latest.

@cherrydev
Copy link
Author

Okay, so looking into this more, I see that when it's running the modification checker, it compares it to the 'raw' version of the existing database object, without the 'reading' hook having been called on it. So what's happening is that it's comparing the modified object (that has a Moment for its 'date' property) with the db object which has a 'Date' for its property. It then recursively finds the differences between the Date object and the Moment object and decides that the modification required to change the Date into a Moment is to add those 7 properties.

I think a straightforward and more correct way of doing this change checking is to FIRST compare the object (constructor) types of the first level properties. Only if they are the SAME should they recurse into checking the sub-properties of them. If they are different, the first-level object is just replaced with the second.

This seems correct because semantically, there is no way to modify properties of an object of one type to turn it into an object of another type.

I suppose another way of dealing with this issue is to run the original object through the 'reading' hook before passing it to 'updating'.

Thoughts?

@dfahlander
Copy link
Collaborator

I suppose you're right. I'll be looking into the function getObjectDiff() in utils.js to see if it should be improved.

dfahlander added a commit that referenced this issue May 10, 2016
@dfahlander dfahlander mentioned this issue May 10, 2016
dfahlander added a commit that referenced this issue May 10, 2016
@dfahlander
Copy link
Collaborator

dfahlander commented May 10, 2016

You could try 1.4.0-beta.3 which fixes this issue.

npm install dexie@next

or

bower install dexie@1.4.0-beta.3

@cherrydev
Copy link
Author

I'll try that. Thanks very much, that was very responsive!

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

2 participants