Supporting multiple tables
Opened this issue · 10 comments
What do you think about allowing different tables within a batch operation (and therefore making clear
a meaningful option also)?
Agreed, with development of Safari, having only one store support does not make big sense. I'd like to see a PR for this feature.
I'd suggest batch(db, object)
syntax as default, instead of current batch(db, string, array)
:
await batch(db, {
magazines: [
{ type: 'add', key: 1, value: { name: 'M1', frequency: 12 } },
{ type: 'add', key: 2, value: { name: 'M2', frequency: 24 } },
{ type: 'add', key: 3, value: { name: 'M3', frequency: 6 } },
{ type: 'del', key: 4 },
]
})
One possible issue is transaction rollback, which might not work everywhere.
That'd be close to my ideal API too, unless you were open to reducing redundancy even a little further:
await batch(db, {
magazines: {
add: [
{ key: 1, value: { name: 'M1', frequency: 12 } },
{ key: 2, value: { name: 'M2', frequency: 24 } },
{ key: 3, value: { name: 'M3', frequency: 6 } }
],
del: [{key: 4}],
},
oldMagazines: null // to clear out a store (reminiscent of JSON Patch Merge)?
})
Why wouldn't rollback work everywhere though? It wouldn't make sense for e.target.transaction
on the upgradeneeded
event to be scoped to only a particular store, and the spec states, under "steps for running a "versionchange" transaction":
The scope of the transaction includes every object store in
connection
.
(and the draft spec also states within "upgrade transaction steps"):
Let
transaction
be a new upgrade transaction withconnection
used as connection. The scope oftransaction
includes every object store in connection.
(The regular IDBDatabase.transaction()
allows for an array to be passed with multiple store names, though one cannot use that within upgradeneeded
since multiple simultaneous transactions are prohibited).
I amended the post just now to supply a possible API for running clear()
on stores since it doesn't need the normal full API, maybe setting to null
(or perhaps the string "clear") would be sufficient.
Though also allowing type: 'clear'
wouldn't hurt either...
I took array notation from node-leveldb. It's not ideal, but at least more people can understand it. With a simple wrapper function you can convert it to { add, put, del }
or embedded object notation, which I would like to remove in next major version.
Let's discuss clear
in separate issue, with this addition it can become full featured library to manage transactions.
I have been thinking about this, maybe instead of extending idb-batch
, we can make it more flexible and allow to accept transaction and callbacks? Because the main problem is to reuse transaction.
Current batch(db, storeName, opts)
function returns Promise and accepts 3 arguments. I suggest to add transactionalBatch(tr, storeName, opts, cb)
(maybe with different name), which accepts 4 arguments and uses callback API.
The deal with multiple transactions, that we (as web dev community) are quite far from the world were we can reuse transactions (thanks Safari) and current idb-batch approach could be more reliable.
In this case, you can implement a library that uses transactionalBatch
and manages transactions completely (idb-transaction?). Check:
import { open } from 'idb-factory'
import Schema from 'idb-schema'
import { transactionalBatch } from 'idb-batch'
const Schema = new Schema()
.addStore('books')
.addStore('magazines')
.addStore('trash')
const db = await open('mydb', schema.version(), schema.callback())
// new library does this handling
const tr = db.transaction(['books', 'magazines', 'trash'], 'readwrite')
transactionalBatch(tr, 'books', [
{ type: 'add', key: 1, value: { name: 'B1' } },
{ type: 'add', key: 2, value: { name: 'B2' } },
{ type: 'add', key: 3, value: { name: 'B3' } },
], (err) => {
// handle err
transactionalBatch(tr, 'magazines', [
{ type: 'put', key: 1, value: { name: 'M1', frequency: 12 } },
], (err) => {
// handle err
tr.objectStore('trash').clear() // handle error and wait transaction to complete
})
})
I'm not sure what you mean with reusing transactions. I've seen that transactions expire (when using promises within upgradeneeded
), but that's not required to do multiple store modifications in one transaction (though promises do need to be avoided).
Also, are you suggesting it is necessary to call transactionalBatch
more than once because Safari doesn't allow multiple store changes within a transaction? That sounds like a very bad implementation problem in Safari if that is the case. So you are saying your idea to change to this API wouldn't work?:
const tr = db.transaction(['books', 'magazines', 'trash'], 'readwrite')
transactionalBatch(tr, {
magazines: [
{ type: 'add', key: 1, value: { name: 'M1', frequency: 12 } },
{ type: 'add', key: 2, value: { name: 'M2', frequency: 24 } },
{ type: 'add', key: 3, value: { name: 'M3', frequency: 6 } },
{ type: 'del', key: 4 },
],
books: [
{ type: 'put', key: 1, value: { name: 'M1', frequency: 12 } }
],
oldBooks: 'clear'
}, (err) => {
tr.objectStore('trash').clear()
})
A transaction is supposed to be able to support multiple store changes as long as the transaction it is added on is an array of the stores...
And for upgradeneeded
usage, it could just look like:
transactionalBatch(e.target.transaction, {
magazines: [
// ....
})
Btw, in place of your object notation, what do u think of this?
await batch(db, 'storage', {
key1: 'update value',
key2: '\0', // delete value
key3: 'new value',
key4: null, // add null value
key5: '\0\0' // add single \0
key6: '\0\0\0' // add \0\0
})
'\0' is as many characters to type as null
, but it is less commonly used, and this approach lets you even get \0
added too if you wish. I've used this approach in my PR just now for db.js, so just thought I'd share.
Also, are you open to an added utility function (or even just auto-detecting the presence of add
, etc. as properties) to support this format:
add: [
{ key: 1, value: { name: 'M1', frequency: 12 } },
{ key: 2, value: { name: 'M2', frequency: 24 } },
{ key: 3, value: { name: 'M3', frequency: 6 } }
]
FYI, I've got a PR coming to add transactionalBatch
(which should be soon after I confirm it is working in my real world needs with db.js). In the process,I have refactored batch
to rely on transactionalBatch
.
Just wanted to let you know if it might save you some work if you were going to try doing it yourself.