I've got quite a large number of changes here, but I think they are a bit interrelated (mostly on the theme of yet more comprehensive promise rejection, validation, and ensuring Firefox can now pass all tests; the bad-args.js test file gives a practical indication of what can be caught, some of which were not catchable before).
Below is a detailed summary of all of the changes in this PR. There is slightly less detail in the updated CHANGES file (avoiding mention of very minor or relatively inconsequential internal refactoring changes which I nevertheless mention below).
There were a few refactoring changes which may make it appear larger than it was though, such as switching to const/let.
Note that I tried to be conservative in extending testing timeouts for Firefox, only adding it where it was noted as necessary as per my testing results.
I was also conservative in adding try-catch blocks for the hard (but catchable) errors I could replicate, but I tried to be fairly thorough, but I may have missed some of these.
Here are the changes:
Breaking change: Require db.cmp to use promises (so errors can be caught in analogous fashion);
Breaking change (minor): Return error object with Promise reject calls;
Breaking change (minor): Change bad keys error message;
Deprecated: on schema.indexes, in place of the index key property, keyPath should be used;
API addition: Add aliased server methods put and delete (for closer parity with spec);
API change: Allow add/update {item:...} without key for sake of unambiguity;
API change: Allow add/update {item:...} to be of any value including undefined or null;
API change: Allow Mongoifying of add/update/remove keys;
API change: Disallow key in count() if null;
Cross-browser support: Auto-wrap user-supplied Server.error() and Server.addEventListener('error', ...) handlers with preventDefault so as to avoid hard ConstraintError aborts in Firefox;
Cross-browser support: move rejected errors' execution from onupgradeneeded to onsuccess (Firefox);
Cross-browser support (minor): wrap deleteonblocked event's newVersion (=null) with Proxy but avoid using using Proxy if not present for sake of PhantomJS or older browsers (Firefox) - fix/improve comment wrapper for oldVersion but could not wrap it, however, so avoid testing (not part of our API anyways but do test err.type is blocked);
Fix (error checking): Reject (and break) upon errors (createObjectStore, bad index creation, bad function on modify() object, bad modify result, bad map() function, and bad put()), and test for such bad argument promise catching as well as testing regular try-catching for event listener methods;
Fix: Ensure limit() arguments are numeric (and test);
Fix: Add in missing (though already documented) chaining of (short) event handlers;
Fix: Allow empty string indexes;
Fix (data validation): Tighter checking on argument to modify method (ensure is an object);
Testing (Gruntfile): Add "phantom" task (separate task but still run by npm test); avoid displaying Saucelabs or adding testJobs when not needed (e.g., for "dev"); document
Testing: get Firefox to avoid errors by lengthening test timeouts (adding as needed)
Docs: Clarify db.delete() "success" on non-existent database, "schema" property, modify
Document add/update's key/item properties and flattening behavior
Refactoring: Reorder Server to be after IndexQuery (since invoking it) and put Server.query at top so it can be closer to IndexQuery which it calls
Refactoring: Use const/let where possible (and make consistent and grouped together)
Refactoring: Exit loop upon Promise rejection for add()/update();
Refactoring: Comment non-throwing code to indicate why certain potentially throwing code is not being wrapped in try-catch
Refactoring (minor): complete simplifying of object keys with same name as variable in scope, capitalize comments, iterate using Object.keys to avoid need for hasOwn checks
Refactoring (minor): Move key conversion to top of function (so can exit earlier if problematic);
Refactoring (minor): Remove unused second argument accidentally supplied to resolve
Hi Aaron,
I've got quite a large number of changes here, but I think they are a bit interrelated (mostly on the theme of yet more comprehensive promise rejection, validation, and ensuring Firefox can now pass all tests; the
bad-args.js
test file gives a practical indication of what can be caught, some of which were not catchable before).Below is a detailed summary of all of the changes in this PR. There is slightly less detail in the updated CHANGES file (avoiding mention of very minor or relatively inconsequential internal refactoring changes which I nevertheless mention below).
There were a few refactoring changes which may make it appear larger than it was though, such as switching to const/let.
Note that I tried to be conservative in extending testing timeouts for Firefox, only adding it where it was noted as necessary as per my testing results.
I was also conservative in adding try-catch blocks for the hard (but catchable) errors I could replicate, but I tried to be fairly thorough, but I may have missed some of these.
Here are the changes:
db.cmp
to use promises (so errors can be caught in analogous fashion);schema.indexes
, in place of the indexkey
property,keyPath
should be used;put
anddelete
(for closer parity with spec);{item:...}
withoutkey
for sake of unambiguity;{item:...}
to be of any value includingundefined
ornull
;add
/update
/remove
keys;count()
if null;Server.error()
andServer.addEventListener('error', ...)
handlers withpreventDefault
so as to avoid hardConstraintError
aborts in Firefox;onupgradeneeded
toonsuccess
(Firefox);delete
onblocked
event'snewVersion
(=null) withProxy
but avoid using usingProxy
if not present for sake of PhantomJS or older browsers (Firefox) - fix/improve comment wrapper foroldVersion
but could not wrap it, however, so avoid testing (not part of our API anyways but do testerr.type
isblocked
);createObjectStore
, bad index creation, bad function onmodify()
object, badmodify
result, badmap()
function, and badput()
), and test for such bad argument promise catching as well as testing regular try-catching for event listener methods;limit()
arguments are numeric (and test);modify
method (ensure is an object);createIndex
validating (issue #149)npm test
); avoid displaying Saucelabs or adding testJobs when not needed (e.g., for "dev"); documentdb.delete()
"success" on non-existent database, "schema" property,modify
add
/update
'skey
/item
properties and flattening behaviorServer
to be afterIndexQuery
(since invoking it) and putServer.query
at top so it can be closer toIndexQuery
which it callsconst
/let
where possible (and make consistent and grouped together)add()
/update()
;Object.keys
to avoid need forhasOwn
checksresolve