jakearchibald / idb

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

Protections against awaiting asynchronous promises breaking transactions #122

Open justjake opened 5 years ago

justjake commented 5 years ago

I'm considering adopting IDB (or another promise-based IndexedDB wrapper) for my project, but am concerned about the danger of encouraging/allowing awaiting of arbitrary promises that break transactions.

To mitigate the risk, an IndexedDB wrapper library could return a special branded Promise type (say, TransactionalOperationPromise), and also provide an eslint-typescript rule that forbids code within an IndexedDB transaction context from awaiting any type other than TransactionalOperationPromise. An "IndexedDB transaction context" could be defined as a lexical scope that references a value that is only valid for the duration of a transaction, for instance an ObjectStore. Or it could be any lexical scope that calls a function that returns a TransactionalOperationPromise -- I haven't though through the specifics of this part.

To allow for async function declarations, the wrapper would export an "enhancer" no-op function transactional<I extends any[], O>(unwrapped: fn(args: ...I): Promise<O>): fn(args: ...I): IDBTransactionalOperationPromise<O> recognized by the linter rule as putting the unwrapped function argument in "transaction" context. The linter rule would only allow a function literal to be passed to transactional(...).

I don't think it's possible to enforce soundness with this tactic, but perhaps eslint-typescript rule that makes some soundness tradeoffs could be both reasonable to implement, and catch a fair amount of bugs. See the await-thenable rule for an example of an eslint-typescript rule that leverages type information.

Does this sound like a promising (no pun intended) track of work to you all? Would you accept a PR implementing this concept in IDB?

justjake commented 5 years ago

I implemented a first draft of the typescript-eslint rule against a modified version of IDB, which confirms that this idea is workable. I hope to polish my implementation and eventually release a configurable ESLint plugin that could be used with both Dexie and IDB.

To make the checking work, I perform the following edits to idb/build/esm/entry.d.ts via a post-install script in my project:

  1. Insert this new type: export type IDBPromise<T> = Promise<T> & { readonly __idb__: 'IDB promises must be micro-tasks' }
  2. Find and replace all Promise with IDBPromise
jakearchibald commented 4 years ago

This feels pretty difficult to get right, but I'm interested to see anything you've got.