Skip to content

Support for browser platform (with WebSQL).#575

Closed
rafw87 wants to merge 3 commits into
brody2consult:storage-masterfrom
rafw87:browser
Closed

Support for browser platform (with WebSQL).#575
rafw87 wants to merge 3 commits into
brody2consult:storage-masterfrom
rafw87:browser

Conversation

@rafw87

@rafw87 rafw87 commented Oct 22, 2016

Copy link
Copy Markdown

This is a simple implementation of browser platform, using WebSQL.
It doesn't have implementation of .close() and .deleteDatabase(), these methods are calling success() but are doing nothing (except writing warning to log). Due of it, selfTest is passing only once and reguires to clear database manually.
EDIT: second commit implements .deleteDatabase() by deleting all tables - it's enough to fix selfTest.

@brody2consult

Copy link
Copy Markdown
Owner

Hi @rafw87 thanks for the contribution. Did you run it against the full test suite?

Looking at the changes I don't think it will pass the error handling tests. Please correct me if I am mistaken somehow. To be honest I was considering a much different approach.

@rafw87

rafw87 commented Oct 25, 2016

Copy link
Copy Markdown
Author

I checked this error handling tests and I see that most of errors are caused by different error messages etc than expected, for example:
Error: Expected 'Cannot read property 'toString' of undefined' to match /Unable to get property 'toString' of undefined or null reference/.

I see that there are different expectations for different environments (Windows, Android, WP8), so maybe it should be done that way for browser platform (expectations like for isWebSql)?

@rafw87

rafw87 commented Oct 25, 2016

Copy link
Copy Markdown
Author

Hi,
I'll check it later today. Both CI jobs passed, I assume that full test suite is something else?

It's possible that some cases are not handled properly. I didn't spend so much time for this platform, it was rather a quick change to have code in my project more clean. I decided to share it "as-is", maybe it will be useful for someone.

Thanks for this plugin:)

-------- Oryginalna wiadomość --------
Od: Chris Brody notifications@github.com
Data:25.10.2016 10:21 (GMT+01:00)
Do: litehelpers/Cordova-sqlite-storage Cordova-sqlite-storage@noreply.github.com
DW: rafw87 raf_w87@wp.pl, Mention mention@noreply.github.com
Temat: Re: [litehelpers/Cordova-sqlite-storage] Support for browser platform (with WebSQL). (#575)
Hi @rafw87 thanks for the contribution. Did you run it against the full test suite?

Looking at the changes I don't think it will pass the error handling tests. Please correct me if I am mistaken somehow. To be honest I was considering a much different approach.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@brody2consult

Copy link
Copy Markdown
Owner

Both CI jobs passed, I assume that full test suite is something else?

The CI jobs test the Android and iOS platform implementations. There is no CI for the browser platform since it would be new. (Also there is currently no CI for Windows.)

I decided to share it "as-is", maybe it will be useful for someone.

Thanks for sharing. It was definitely very interesting to try running it.

I think it will take a very different approach to pass some of the more difficult tests including nested transactions, error handling, and error recovery. I will work on this when I get a chance. Discussion will be continued in #297.

Thanks for this plugin:)

You are welcome)

@brody2consult

Copy link
Copy Markdown
Owner

Closing in favor of PR #576

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants