storesafe/cordova-sqlite-storage

Deprecate standard transaction mechanism in next major release?

brodycj opened this issue · 8 comments

brodycj commented

The deprecated Web SQL transaction API described in https://www.w3.org/TR/webdatabase/ has led to some bugs and significant confusion over the past few years. For example:

  • #409 - TypeError in success callback, fixed with a workaround solution
  • plugin would fire more callbacks after an error with no error handler that returns false (fixed with no issue filed)
  • #666 - Transaction problem after page change, WITH POSSIBLE DATA LOSS (fixed with a workaround solution in this plugin version)
  • litehelpers/cordova-sqlite-evmax-ext-workers-legacy-build-free#3 - transaction within a transaction issue with simultaneous transactions in web worker and main app code in that version (workaround is to use database.executeSql and database.sqlBatch instead of database.transaction in both worker and main app code)

ADDITIONAL ISSUES DISCOVERED:

Given these issues I would like to deprecate the db.transaction call in the next major release discussed in #687. Users would be advised to use db.executeSql for SELECT and single-statement changes and database.sqlBatch for multi-statement changes as already documented. These calls would be supported on the "browser" platform in addition to Android/iOS/macOS/Windows.

I am thinking of the following options:

  • Require a special setting in sqlitePlugin.openDatabase if the user wants to use the standard db.transaction call
  • The db object constructed by the sqlitePlugin.openDatabase call would not support the standard db.transaction call. Instead the user could issue a call such as db.getStandardDatabaseHandle() to get a new object that does support the db.transaction call.

I would like to leave this issue open for feedback from the user community.

ADDITIONAL NOTE: This may go along with be related to the possible redesign discussed in #548.

abram456 commented

Require a special setting in sqlitePlugin.openDatabase if the user wants to use the standard db.transaction call!!!! What is that special setting @brodybits ??

brodycj commented

What is that special setting @brodybits ??

Not decided yet.

kanoshin commented

In some cases it is useful to do several actions in one transaction that depend on each other. E.g. read some rows, perform calculation and then update this rows in the same transaction. Looks like new API doesn't allow it.

mirko77 commented

I am using db.transaction a lot to be fair, how are we handling backwards compatibility? By using that flag you mentioned? @brodybits

RhinoLance commented

As Chrome doesn't appear to support db.executeSql, this change will remove the ability to develop and test against a browser before moving to a device. That was always the strength of the plugin, in that I could transparently fall back to WebSQL if required.
The issues does state "These calls would be supported on the "browser" platform...", but it doesn't work for me. Am I missing something?

brodycj commented

Thanks guys for the feedback, will not be removed from this plugin version.

iamjoyce commented

@brodybits Does that mean that db.transaction may still be removed in future versions? In that case, is it recommended to just use executeSql where possible instead? Thanks

brodycj commented

@brodybits Does that mean that db.transaction may still be removed in future versions?

Not from this plugin version. May not be supported by a new sqlite plugin based on possible redesign discussed in #548.

[...] is it recommended to just use executeSql where possible instead?

I would recommend db.executeSql() call for read and simple (single-statement) write or db.sqlBatch() call in case of multiple-statement modification. Simpler, more future-proof in case of new sqlite plugin version with redesign discussed in #548.

FYI a promise-based API wrapper is available in https://github.com/brodybits/sql-promise-helper, which provides functionality similar to db.executeSql() and db.sqlBatch() calls.