Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing Web SQL transaction calls from next major release #720

Closed
brodycj opened this issue Oct 26, 2017 · 9 comments
Closed

Removing Web SQL transaction calls from next major release #720

brodycj opened this issue Oct 26, 2017 · 9 comments

Comments

@brodycj
Copy link
Contributor

brodycj commented Oct 26, 2017

Due to the number of issues and level of confusion encountered with the "standard" (deprecated) Web SQL transaction API discussed in #690, especially for multi-page apps: I would like to remove support for these calls (db.transaction() and db.readTransaction()) from the next major release of this plugin version ref: #687, concurrent with addition of browser platform support ref: #297.

NOTE that these calls would NOT be removed from other plugin versions such as cordova-sqlite-ext or cordova-sqlite-evcore-extbuild-free. In case these calls are still needed possible solutions would include installing cordova-sqlite-ext or cordova-sqlite-storage@2.

I would like to leave this open for feedback. In case of objections I would appreciate a very clear statement of why we should not remove these calls from this plugin version and why using cordova-sqlite-ext would not solve the problem.

@brodycj
Copy link
Contributor Author

brodycj commented Oct 26, 2017

To clarify: it is recommended to use the following calls in general:

  • db.executeSql() call to read data or execute a single statement
  • db.sqlBatch() to execute a batch of modification statements within an ACID (atomic, failure-safe) transaction.

Using these calls will help avoid issues with multi-page apps ref: #666, #702 and would also be better supported by the evplus workers version ref: litehelpers/cordova-sqlite-evmax-ext-workers-legacy-build-free#3, storesafe/cordova-sqlite-evcore-extbuild-free#11

An additional suggestion is to consider using https://github.com/brodybits/sql-promise-helper to get a nice, clean Promise-based API that also works on the Browser platform today.

@luigi37
Copy link

luigi37 commented Jan 4, 2018

Hi,
am I over simplifying this or it would be enough to replace for example:

db.transaction(function(tx) {
	tx.executeSql('SELECT * FROM myTable WHERE my_ID = ?', [item.my_ID], function(tx, result3) {
		for (var h = 0, item3 = null; h < result3.rows.length; h++) {
		item3 = result3.rows.item(h);
		//do something
		}
	});
});

with

db.executeSql('SELECT * FROM myTable WHERE my_ID = ?', [item.my_ID], function(result3) {
	for (var h = 0, item3 = null; h < result3.rows.length; h++) {
	item3 = result3.rows.item(h);
	//do something
	}
});

?

@brodycj
Copy link
Contributor Author

brodycj commented Jan 4, 2018

@luigi37 that definitely looks right to me, more efficient as well. (I took the liberty to add js highlighting to your code.) Thanks for asking.

FYI I am still not 100% sure about removing the deprecated "standard" transaction calls from this plugin version due to the number of apps that would very likely be broken. My idea right now is to make a lightweight "express" version that uses the builtin sqlite libs on Android/iOS, with the deprecated transaction calls removed from that version.

@luigi37
Copy link

luigi37 commented Jan 4, 2018

Thanks Chris.
I learned something new on GitHub :-)

Cool, that gives me some time :-) Nonetheless I'll try to move asap.

I noticed I added (years ago) to my code a function "executeSqlBridge"
https://gist.github.com/orbitaloop/2009518

In all honestly I can't even recall why.... do you think there is any reason to keep it or I can just replace with tx.executeSql first and then db.executeSql?

I do apologies for the OT point but I think is still somehow related (at least it is related with my codebase migration) and I guess it will take you few seconds to look at the code.

Thanks

@brodycj
Copy link
Contributor Author

brodycj commented Jan 4, 2018

I noticed I added (years ago) to my code a function "executeSqlBridge"
https://gist.github.com/orbitaloop/2009518

The gist was made because the original iOS plugin version by davibe (Davide Bertolla) did not mirror the draft standard API call correctly, no longer needed if you use this plugin version. I did contribute a quick fix in davibe/Phonegap-SQLitePlugin#24 / davibe/Phonegap-SQLitePlugin#23 but it was rejected (davibe/Phonegap-SQLitePlugin#23 (comment)) due to risk of breaking existing apps (back in 2012). General kudos to davibe for the iOS implementation which has proven extremely stable in most cases over the past 6 years.

I also published https://github.com/brodybits/sql-promise-helper which gives you a Promise-based abstraction, uses the more efficient calls if available, and also supports testing on the browser platform TODAY.

Thanks @luigi37 for your interest in my work. Please feel free to ask anytime if you have any more questions, concerns, or ideas.

@luigi37
Copy link

luigi37 commented Jan 4, 2018

Cool, thanks Chris, some less code to handle :-)

As per browser I must admit that I have problems in using it and I rely 99% on Ripple with Chrome (as , unfortunately, Firefox dropped websql support)

Thanks also for your availability and your great work which makes so many of our apps possible :-)

@luigi37
Copy link

luigi37 commented Jan 15, 2018

FYI I am still not 100% sure about removing the deprecated "standard" transaction calls from this plugin version due to the number of apps that would very likely be broken.

Hi Chris, I realized there is a massive amount of libraries which relies on old STD transaction. So... in effect, I would recommend in doucmentation to NOT use the standard transaction but I would not remove it...

Do you agree?

@brodycj
Copy link
Contributor Author

brodycj commented Jan 16, 2018

Do you agree?

Hi @luigi37, unfortunately I think I have to agree with you. Deprecated draft Web SQL transaction API will stay in this plugin version, will possibly be removed from new express version (#740).

@brodycj
Copy link
Contributor Author

brodycj commented Jan 17, 2018

Will not be removed from this plugin version, closing now.

@brodycj brodycj closed this as completed Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants