jakearchibald / idb

IndexedDB, but with promises
https://www.npmjs.com/package/idb
ISC License
6.29k stars 353 forks source link

Add mode to IDBPTransaction #200

Closed zhouhanseng closed 3 years ago

zhouhanseng commented 3 years ago

Fixes https://github.com/jakearchibald/idb/issues/140

  1. Add type check for transaction's mode, it is impossible to pass a readonly transaction to a function that requires a readwrite transaction.

  2. Exclude add(), put(), delete(), clear() from objectstore created by readonly transaction. Keep createIndex() only in objectstore created by versionchange transaction.

const schemaDB = await openDBWithSchema();
const db = schemaDB as IDBPDatabase<TestDBSchema>;

const t1 = db.transaction("store1", "readonly") 
const t2 = db.transaction("store1", "readwrite") 

const action = (t: IDBPTransaction<TestDBSchema, ['store1'], "readwrite">) => {
  t.store.clear()
}

// ts compiler: Type '"readonly"' is not assignable to type '"readwrite"'
action(t1)
// ts compiler: IDBPObjectStore<TestDBSchema, ["store1"], "store1", "readonly">.add: undefined
t1.store.add({name: 'm'})
// ts compiler: IDBPObjectStore<TestDBSchema, ["store1"], "store1", "readwrite">.createIndex: undefined
t2.store.createIndex('name', 'name')
zhouhanseng commented 3 years ago

Updated the test.

zhouhanseng commented 3 years ago

Updated the test.

jakearchibald commented 3 years ago

Excited about this. I'll review the change soon and get it merged. I guess this will be a new major version for safety.

zhouhanseng commented 3 years ago

Is this PR okay?

jakearchibald commented 3 years ago

I want to give it some detailed testing before I land it, but it looks good at a glance. I also need to figure out if it requires a major version bump or not. Sorry for the delay, just not got a bunch of free time right now. It's still on my todo list, and I really appreciate the work you've put in.

jakearchibald commented 3 years ago

Sorry I still haven't gotten to this. It's still on my TODO list, I've just hit a bunch of deadlines.

jakearchibald commented 3 years ago

Don't worry about the conflicts, I'll solve those

jakearchibald commented 3 years ago

@zhouhanseng I finally found time to review this. Excellent work. I'm going to pick up a couple of other bug reports and publish a new version.

Jack-Works commented 3 years ago

hi @zhouhanseng, can a ReadWrite tx assign to a ReadOnly tx? That is a safe action.

Jack-Works commented 3 years ago

Store names should also come in the check:

The type checking rule should be:

  1. If A contains more IDB stores than B, B = A should pass, A = B should error
  2. If A is RW and B is RO, B = A should pass, A = B should error
zhouhanseng commented 3 years ago

@Jack-Works No, it can't assign.

Jack-Works commented 3 years ago

@zhouhanseng it should be, it's runtime safe so it should pass the compiler.