Skip to content
This repository has been archived by the owner on Jun 15, 2022. It is now read-only.

fix: device interface #273

Closed
wants to merge 3 commits into from
Closed

fix: device interface #273

wants to merge 3 commits into from

Conversation

johnny243
Copy link
Contributor

@johnny243 johnny243 commented Jul 24, 2020

SNJS expects raw interface functions to return raw strings (if any).

Related: standardnotes/snjs#52

src/lib/interface.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@radko93 radko93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overal good, just check on comments about stringifying.

@moughxyz
Copy link
Member

Ok so here's the thing: if mobile's implementation of setRawStorageValue JSON.stringifies the value as an implementation detail, then mobile's implementation of getRawStorageValue should also JSON.parse automatically, as an implementation detail. This would make it so that whatever SNJS gives as a raw value is the exact format also retrieved by SNJS.

So if AsyncStorage only accepts strings, but setRawValue takes in any, then the implementation should JSON.stringify automatically.

You don't need to check if a value is already stringified, since double-stringifying is ok as long as we also double parse.

As it stands I'm not sure if these changes will just be backward compatible with the existing setup? Were you able to confirm this?

@moughxyz
Copy link
Member

Also any private functions like getRawStorageKeyValues do not need to be modified as these are not consumed by SNJS. So the implementation should be whatever is most convenient for the mobile codebase.

@johnny243
Copy link
Contributor Author

johnny243 commented Jul 24, 2020

I think that at least we could standardize the device interface. For example: web interface get and set raw values (no JSON parsing and/or stringify inside).

As it stands I'm not sure if these changes will just be backward compatible with the existing setup? Were you able to confirm this?

Tested by upgrading to 3.5.0 from 3.0.22. So it seems backward compatible at least with that version I guess.

@moughxyz
Copy link
Member

What each platform does is up to them. The only thing SNJS cares about is the return value. What the platforms do inside to compute that return value is up to them.

@johnny243
Copy link
Contributor Author

Got it 👍 closing this for now

@johnny243 johnny243 closed this Jul 28, 2020
@johnny243 johnny243 deleted the 004-fix-device-interface branch July 28, 2020 03:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants