(not working) Try having ZulipAsyncStorage class extend AsyncStorage class.#4947
Closed
chrisbobbe wants to merge 5 commits intozulip:masterfrom
Closed
(not working) Try having ZulipAsyncStorage class extend AsyncStorage class.#4947chrisbobbe wants to merge 5 commits intozulip:masterfrom
ZulipAsyncStorage class extend AsyncStorage class.#4947chrisbobbe wants to merge 5 commits intozulip:masterfrom
Conversation
The types that ship with the library are pretty useless; see node_modules/@react-native-async-storage/async-storage/lib/commonjs/AsyncStorage.js.flow. So, check our definitions against the API doc at https://react-native-async-storage.github.io/async-storage/docs/api. That doc isn't very specific either, but at least it confirms that `getItem` should return a Promise that resolves to `null` or `string`, and in a roundabout way it confirms that `getAllKeys` returns a Promise that resolves to an array of `string`s. For others, it doesn't say what the returned Promise resolves to, so we say `mixed` for those.
We'd been trying to do this by putting a line in `[libs]`, under the
following comment:
```
; If the following files are not explicitly named, Flow ignores them
; in favor of their autogenerated versions in flow-typed/npm.
```
But the problem isn't that it's getting shadowed by an autogenerated
stub in flow-typed/npm. The problem is that the Flow types that the
library ships with aren't very good, and we want to ignore them and
use our own libdef. For an example, see `getItem` in
node_modules/@react-native-async-storage/async-storage/lib/commonjs/AsyncStorage.js.flow:
```
static getItem(key: string, callback?: Function): Promise<*> {
return createPromise(() => {
return window.localStorage.getItem(key);
}, callback);
}
```
So, do a thing that accomplishes that.
To strengthen ZulipAsyncStorage's relationship with AsyncStorage in a convenient, readable way.
Types-First apparently can't infer these in its quick skim. One reasonable thing it might have done is consult the base class AsyncStorage. Oh well. At least we get an error if we try to write down a return type that's incompatible with the corresponding method on AsyncStorage.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/ZulipAsyncStorage.20extends.20AsyncStorage.20.28.3F.29/near/1244517.
Note that this is broken when you try to run the app: we'd need to apply a hack to live, in-app code, like we did in jestSetup.js, if we really wanted this style to work.
I'll close this after a brief discussion; I don't think we really want to use this style. But in case there's anything interesting here.