search

aaronpowell / db.js

db.js is a wrapper for IndexedDB to make it easier to work against
http://aaronpowell.github.com/db.js/
MIT License
820 stars 142 forks source link

Documenting any passing of `this` for `Server` methods #176

Closed brettz9 closed 8 years ago

brettz9 commented 8 years ago

I noticed that in Server.add and Server.update, you have the following:

transaction.oncomplete = () => resolve(records, this);

Did you intend to pass this in there? If so, should we not add it to the other methods and document it?

aaronpowell commented 8 years ago

It would appear that you submitted a PR with that particular change (https://github.com/aaronpowell/db.js/commit/6a0e29450b84af39a6c816b01ff9712c00e8f7f0#diff-f786454c0651c074b51fa96e1b531414R80).

From what I understand of the promise API is that only the 1st argument to the resolve method is consumed, so the 2nd argument, this, does nothing.

Testing this code snippet:

new Promise(resolve => 
    setTimeout(() => resolve(1, 2, 3, 4), 0)
).then(function () {
    console.dir(arguments);
    console.log(this === window);
});

Yields only a single argument and true (in Edge and Chome, FF yields all arguments).

brettz9 commented 8 years ago

Whoops, sorry about that--I figured that was the case about Promises--not sure why I had added that like that. Amended in my latest PR. Thanks!