search

TheCocoaProject / cordova-plugin-nativestorage

Cordova plugin: Native storage of variables in Android, iOS and Windows
http://thecocoaproject.github.io/
Apache License 2.0
292 stars 106 forks source link

fix: JSON.stringify of string was adding add additional quotes #124

Closed libinvarghese closed 6 years ago

libinvarghese commented 6 years ago

Currently setItem(key, "stringValue") will store the value "\"stringValue\"" into key.

This commit avoids JSON.stringify if value is of type string.

Since this is a generic method, that is expected to support all value type and is the only setter method supported in @ionic-native, this commit helps in storing string values.

alokrajiv commented 6 years ago

TESTS SUITES FAILING.

The string spec specifically!

It breaks functionality and from my basic look: This commit is not complete. I can see that you have updated the setter but not the getter, which quite possibly is the reason.

alokrajiv commented 6 years ago

Btw to add on, I think setItem currently is able to add and retrieve strings correctly now. What you are trying to do is an enhancement :)

What you are trying to do can save us one cycle of stringify/parse but I don't think the performance impact here is high enough except for extremely large volumes of read/writes per second. (though I think I/O will be your harder hitting bottleneck)

Also, the getter complexity might increase drastically because different platforms have different languages and datatype support underneath - ObjC, Java etc which could possibly increase the complexity that side. (this part is the missing part of this merge) .

@GillesC we discussed this from v1 --> v2. Do, comment if you think we should go this route.

alokrajiv commented 6 years ago

Do note: Please complete & test the branch on your fork, before sending in a PR :)

Unfortunately, I have to close this request now.

If you feel otherwise, simply open an issue with your thoughts on why you need this feature (feature-request/enhancement), we can discuss and help you with a new PR :)