fix: Errors reported from getAllKeys/clear [WINDOWS] #424
Conversation
`getAllKeys` was reporting the correct results, but also an empty array error. Unlike the multi* methods, these expect a single error, not an array of errors. The fix is to change the type sent by the windows implementation from an array to a single object. That object is also just a null value, since there are no errors being reported currently by the windows implementation.
| @@ -478,7 +478,7 @@ void DBStorage::DBTask::getAllKeys(sqlite3* db) { | |||
| void DBStorage::DBTask::clear(sqlite3* db) { | |||
| if (Exec(db, m_callback, u8"DELETE FROM AsyncLocalStorage")) { | |||
| std::vector<winrt::JSValue> callbackParams; | |||
| callbackParams.push_back(winrt::JSValueArray()); | |||
| callbackParams.push_back(nullptr); | |||
There was a problem hiding this comment.
you could pass JSValue::Null or even omit the nullptr altogether
There was a problem hiding this comment.
@chrisglein any chance you could take a look on this suggestion?
There was a problem hiding this comment.
Thanks @chrisglein 💪
Let me know if you're done with PR, so I can merge this
|
Since this is not a blocker, I'll going ahead and merge it |
|
Hello, while clear() does erase the contents of the AsyncStorage.db file, it does return an Error - literally just printing "Error" in my console. Similar case with getAllKeys() - here, it returns "Error" without returning the keys. |
|
@exotexot if you're seeing a bug could you please file an issue? Thanks! CC @chrisglein |
|
I don't believe there's been a release with this fix yet. The latest release was from Aug 17 and this was merged after that. I'm not sure what release schedule the module is on - looks pretty sparse. In the meantime you can patch the source files directly (which is what I did to test the fix). |
…nc-storage#424) `getAllKeys` was reporting the correct results, but also an empty array error. Unlike the multi* methods, these expect a single error, not an array of errors. The fix is to change the type sent by the windows implementation from an array to a single object. That object is also just a null value, since there are no errors being reported currently by the windows implementation.
Summary:
On Windows
getAllKeyswas reporting the correct results, but also an empty array error. Unlike the multi* methods, these expect a single error, not an array of errors.The fix is to change the type sent by the windows implementation from an array to a single object. That object is also just a null value, since there are no errors being reported currently by the windows implementation.
Test Plan:
Manually validated with the repo's example app on Windows. Also tested by patching to a react-native-windows app that was already using async storage and failing on
getAllKeys.The test app doesn't contain any usage of
getAllKeys, which is probably why this was missed.