Added initStorage to driver's API. Only creates websql database if really needed. #99
Conversation
|
Neat that this passes all tests already. Should we add some that ensure the databases are only created on demand? How would you ensure that? |
|
There is no way to simply check if a database exists, but we can call openDatabase with a creation callback in the 5th parameter, that is called when the database is being created. The problem is nothing is called when the database already exists. So casper will wait for the callback in one case and wait for a timeout in another? I'm not seeing it clear now, but I'll think bit more. Meanwhile I've committed the tests that checks for the new initStorage method. |
| // Open the database; the openDatabase API will automatically create it for | ||
| // us if it doesn't exist. | ||
| var db = window.openDatabase(DB_NAME, DB_VERSION, STORE_NAME, DB_SIZE); | ||
| function initStorage(callback) { |
There was a problem hiding this comment.
Should this be exposed like it's part of the public API? Does any developer have cause to call this on their own, or is it localForage-only? If so, maybe we can advertise its private-ness with a leading underscore or something.
|
Largely this looks really good. Can you check out my few comments? After that I think it's good to merge in. Nice work. |
|
Done. As for the tests, is hard to do a negative test case. We would be testing what not happens when using other driver (websql db doesn't exists if indexdb is used, indexdb store doesn't exists if localstorage is used, etc.). The positive test case is already covered, as test.api would fail if the correct db or store isn't created. |
|
Ah, you rule! Thanks so much! That makes sense, thanks for explaining. 👍 |
Added initStorage to driver's API. Only creates websql database if really needed.
WebSQL "localforage" database and respective table were always being created, even when using other driver.
The initStorage() function mechanism allows for similar behavior in other drivers, if needed.
An alternative could be to add a withStore to websql.js, similar to the one in indexeddb.js.
However, I think this is a better solution as it cuts the overhead of checking if db == null in every method call.
LocalStorage driver needs no initialization, so initStorage simply does nothing.
Note: This pull request replaces "Create websql database only if really needed" #79, as I forgot to create a new feature branch.