From 05e380f26c65fc3ee35d859bdc509720491e5843 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 12 Jan 2021 11:12:08 -0800 Subject: [PATCH] Type AsyncStorage Summary: Types the AsyncStorage module with Flow. Changelog: [Internal] Reviewed By: MichaReiser Differential Revision: D25884607 fbshipit-source-id: 7762ebfc68b87e6c5a0d8100fcc564f002061f5c --- IntegrationTests/AsyncStorageTest.js | 50 +++++--- Libraries/Storage/AsyncStorage.js | 114 ++++++++++-------- Libraries/Storage/NativeAsyncLocalStorage.js | 17 +-- .../Storage/NativeAsyncSQLiteDBStorage.js | 17 +-- .../js/utils/RNTesterStatePersister.js | 6 +- 5 files changed, 128 insertions(+), 76 deletions(-) diff --git a/IntegrationTests/AsyncStorageTest.js b/IntegrationTests/AsyncStorageTest.js index 12d9f3d620f..a6039d4b6d5 100644 --- a/IntegrationTests/AsyncStorageTest.js +++ b/IntegrationTests/AsyncStorageTest.js @@ -16,6 +16,7 @@ const {AsyncStorage, Text, View, StyleSheet} = ReactNative; const {TestModule} = ReactNative.NativeModules; const deepDiffer = require('react-native/Libraries/Utilities/differ/deepDiffer'); +const nullthrows = require('nullthrows'); const DEBUG = false; @@ -43,15 +44,32 @@ function expectTrue(condition: boolean, message: string) { } } +// Type-safe wrapper around JSON.stringify +function stringify( + value: + | void + | null + | string + | number + | boolean + | {...} + | $ReadOnlyArray, +): string { + if (typeof value === 'undefined') { + return 'undefined'; + } + return JSON.stringify(value); +} + function expectEqual(lhs, rhs, testname: string) { expectTrue( !deepDiffer(lhs, rhs), 'Error in test ' + testname + ': expected\n' + - JSON.stringify(rhs) + + stringify(rhs) + '\ngot\n' + - JSON.stringify(lhs), + stringify(lhs), ); } @@ -61,7 +79,7 @@ function expectAsyncNoError(place, err) { } expectTrue( err === null, - 'Unexpected error in ' + place + ': ' + JSON.stringify(err), + 'Unexpected error in ' + place + ': ' + stringify(err), ); } @@ -71,7 +89,7 @@ function testSetAndGet() { AsyncStorage.getItem(KEY_1, (err2, result) => { expectAsyncNoError('testSetAndGet/getItem', err2); expectEqual(result, VAL_1, 'testSetAndGet setItem'); - updateMessage('get(key_1) correctly returned ' + result); + updateMessage('get(key_1) correctly returned ' + String(result)); runTestCase('should get null for missing key', testMissingGet); }); }); @@ -81,7 +99,7 @@ function testMissingGet() { AsyncStorage.getItem(KEY_2, (err, result) => { expectAsyncNoError('testMissingGet/setItem', err); expectEqual(result, null, 'testMissingGet'); - updateMessage('missing get(key_2) correctly returned ' + result); + updateMessage('missing get(key_2) correctly returned ' + String(result)); runTestCase('check set twice results in a single key', testSetTwice); }); } @@ -105,8 +123,9 @@ function testRemoveItem() { AsyncStorage.getAllKeys((err, result) => { expectAsyncNoError('testRemoveItem/getAllKeys', err); expectTrue( - result.indexOf(KEY_1) >= 0 && result.indexOf(KEY_2) >= 0, - 'Missing KEY_1 or KEY_2 in ' + '(' + result + ')', + nullthrows(result).indexOf(KEY_1) >= 0 && + nullthrows(result).indexOf(KEY_2) >= 0, + 'Missing KEY_1 or KEY_2 in ' + '(' + nullthrows(result).join() + ')', ); updateMessage('testRemoveItem - add two items'); AsyncStorage.removeItem(KEY_1, err2 => { @@ -123,8 +142,8 @@ function testRemoveItem() { AsyncStorage.getAllKeys((err4, result3) => { expectAsyncNoError('testRemoveItem/getAllKeys', err4); expectTrue( - result3.indexOf(KEY_1) === -1, - 'Unexpected: KEY_1 present in ' + result3, + nullthrows(result3).indexOf(KEY_1) === -1, + 'Unexpected: KEY_1 present in ' + nullthrows(result3).join(), ); updateMessage('proper length returned.'); runTestCase('should merge values', testMerge); @@ -137,13 +156,17 @@ function testRemoveItem() { } function testMerge() { - AsyncStorage.setItem(KEY_MERGE, JSON.stringify(VAL_MERGE_1), err1 => { + AsyncStorage.setItem(KEY_MERGE, stringify(VAL_MERGE_1), err1 => { expectAsyncNoError('testMerge/setItem', err1); - AsyncStorage.mergeItem(KEY_MERGE, JSON.stringify(VAL_MERGE_2), err2 => { + AsyncStorage.mergeItem(KEY_MERGE, stringify(VAL_MERGE_2), err2 => { expectAsyncNoError('testMerge/mergeItem', err2); AsyncStorage.getItem(KEY_MERGE, (err3, result) => { expectAsyncNoError('testMerge/setItem', err3); - expectEqual(JSON.parse(result), VAL_MERGE_EXPECT, 'testMerge'); + expectEqual( + JSON.parse(nullthrows(result)), + VAL_MERGE_EXPECT, + 'testMerge', + ); updateMessage('objects deeply merged\nDone!'); runTestCase('multi set and get', testOptimizedMultiGet); }); @@ -165,8 +188,7 @@ function testOptimizedMultiGet() { expectAsyncNoError(`${i} testOptimizedMultiGet/multiGet`, err2); expectEqual(result, batch, `${i} testOptimizedMultiGet multiGet`); updateMessage( - 'multiGet([key_1, key_2]) correctly returned ' + - JSON.stringify(result), + 'multiGet([key_1, key_2]) correctly returned ' + stringify(result), ); done(); }); diff --git a/Libraries/Storage/AsyncStorage.js b/Libraries/Storage/AsyncStorage.js index d1fd9c23f54..a4fcfd70a98 100644 --- a/Libraries/Storage/AsyncStorage.js +++ b/Libraries/Storage/AsyncStorage.js @@ -5,8 +5,7 @@ * LICENSE file in the root directory of this source tree. * * @format - * @noflow - * @flow-weak + * @flow strict * @jsdoc */ @@ -19,6 +18,20 @@ import invariant from 'invariant'; // Use SQLite if available, otherwise file storage. const RCTAsyncStorage = NativeAsyncSQLiteDBStorage || NativeAsyncLocalStorage; +type GetRequest = { + keys: Array, + callback: ?(errors: ?Array, result: ?Array>) => void, + keyIndex: number, + resolve: ( + result?: + | void + | null + | Promise>> + | Array>, + ) => void, + reject: (error?: mixed) => void, +}; + /** * `AsyncStorage` is a simple, unencrypted, asynchronous, persistent, key-value * storage system that is global to the app. It should be used instead of @@ -27,7 +40,7 @@ const RCTAsyncStorage = NativeAsyncSQLiteDBStorage || NativeAsyncLocalStorage; * See https://reactnative.dev/docs/asyncstorage.html */ const AsyncStorage = { - _getRequests: ([]: Array), + _getRequests: ([]: Array), _getKeys: ([]: Array), _immediate: (null: ?number), @@ -39,7 +52,7 @@ const AsyncStorage = { getItem: function( key: string, callback?: ?(error: ?Error, result: ?string) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiGet([key], function(errors, result) { @@ -65,7 +78,7 @@ const AsyncStorage = { key: string, value: string, callback?: ?(error: ?Error) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiSet([[key, value]], function(errors) { @@ -74,7 +87,7 @@ const AsyncStorage = { if (errs) { reject(errs[0]); } else { - resolve(null); + resolve(); } }); }); @@ -88,7 +101,7 @@ const AsyncStorage = { removeItem: function( key: string, callback?: ?(error: ?Error) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiRemove([key], function(errors) { @@ -97,7 +110,7 @@ const AsyncStorage = { if (errs) { reject(errs[0]); } else { - resolve(null); + resolve(); } }); }); @@ -115,7 +128,7 @@ const AsyncStorage = { key: string, value: string, callback?: ?(error: ?Error) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiMerge([[key, value]], function(errors) { @@ -124,7 +137,7 @@ const AsyncStorage = { if (errs) { reject(errs[0]); } else { - resolve(null); + resolve(); } }); }); @@ -137,7 +150,7 @@ const AsyncStorage = { * * See https://reactnative.dev/docs/asyncstorage.html#clear */ - clear: function(callback?: ?(error: ?Error) => void): Promise { + clear: function(callback?: ?(error: ?Error) => void): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.clear(function(error) { @@ -145,7 +158,7 @@ const AsyncStorage = { if (error && convertError(error)) { reject(convertError(error)); } else { - resolve(null); + resolve(); } }); }); @@ -158,7 +171,7 @@ const AsyncStorage = { */ getAllKeys: function( callback?: ?(error: ?Error, keys: ?Array) => void, - ): Promise { + ): Promise> { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.getAllKeys(function(error, keys) { @@ -229,7 +242,7 @@ const AsyncStorage = { multiGet: function( keys: Array, callback?: ?(errors: ?Array, result: ?Array>) => void, - ): Promise { + ): Promise>> { if (!this._immediate) { this._immediate = setImmediate(() => { this._immediate = null; @@ -237,29 +250,22 @@ const AsyncStorage = { }); } - const getRequest = { - keys: keys, - callback: callback, - // do we need this? - keyIndex: this._getKeys.length, - resolve: null, - reject: null, - }; - - const promiseResult = new Promise((resolve, reject) => { - getRequest.resolve = resolve; - getRequest.reject = reject; + return new Promise>>((resolve, reject) => { + this._getRequests.push({ + keys, + callback, + // do we need this? + keyIndex: this._getKeys.length, + resolve, + reject, + }); + // avoid fetching duplicates + keys.forEach(key => { + if (this._getKeys.indexOf(key) === -1) { + this._getKeys.push(key); + } + }); }); - - this._getRequests.push(getRequest); - // avoid fetching duplicates - keys.forEach(key => { - if (this._getKeys.indexOf(key) === -1) { - this._getKeys.push(key); - } - }); - - return promiseResult; }, /** @@ -271,7 +277,7 @@ const AsyncStorage = { multiSet: function( keyValuePairs: Array>, callback?: ?(errors: ?Array) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiSet(keyValuePairs, function(errors) { @@ -280,7 +286,7 @@ const AsyncStorage = { if (error) { reject(error); } else { - resolve(null); + resolve(); } }); }); @@ -294,7 +300,7 @@ const AsyncStorage = { multiRemove: function( keys: Array, callback?: ?(errors: ?Array) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiRemove(keys, function(errors) { @@ -303,7 +309,7 @@ const AsyncStorage = { if (error) { reject(error); } else { - resolve(null); + resolve(); } }); }); @@ -320,7 +326,7 @@ const AsyncStorage = { multiMerge: function( keyValuePairs: Array>, callback?: ?(errors: ?Array) => void, - ): Promise { + ): Promise { invariant(RCTAsyncStorage, 'RCTAsyncStorage not available'); return new Promise((resolve, reject) => { RCTAsyncStorage.multiMerge(keyValuePairs, function(errors) { @@ -329,7 +335,7 @@ const AsyncStorage = { if (error) { reject(error); } else { - resolve(null); + resolve(); } }); }); @@ -337,24 +343,38 @@ const AsyncStorage = { }; // Not all native implementations support merge. -if (!RCTAsyncStorage.multiMerge) { - delete AsyncStorage.mergeItem; - delete AsyncStorage.multiMerge; +// TODO: Check whether above comment is correct. multiMerge is guaranteed to +// exist in the module spec so we should be able to just remove this check. +if (RCTAsyncStorage && !RCTAsyncStorage.multiMerge) { + // $FlowFixMe[unclear-type] + delete (AsyncStorage: any).mergeItem; + // $FlowFixMe[unclear-type] + delete (AsyncStorage: any).multiMerge; } -function convertErrors(errs) { +function convertErrors( + // NOTE: The native module spec only has the Array case, but the Android + // implementation passes a single object. + errs: ?( + | {message: string, key?: string} + | Array<{message: string, key?: string}> + ), +) { if (!errs) { return null; } return (Array.isArray(errs) ? errs : [errs]).map(e => convertError(e)); } +declare function convertError(void | null): null; +declare function convertError({message: string, key?: string}): Error; function convertError(error) { if (!error) { return null; } const out = new Error(error.message); - out.key = error.key; // flow doesn't like this :( + // $FlowFixMe[unclear-type] + (out: any).key = error.key; return out; } diff --git a/Libraries/Storage/NativeAsyncLocalStorage.js b/Libraries/Storage/NativeAsyncLocalStorage.js index 834cd97f359..39502dde329 100644 --- a/Libraries/Storage/NativeAsyncLocalStorage.js +++ b/Libraries/Storage/NativeAsyncLocalStorage.js @@ -14,29 +14,32 @@ import type {TurboModule} from '../TurboModule/RCTExport'; import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry'; export interface Spec extends TurboModule { - +getConstants: () => {||}; + +getConstants: () => {}; +multiGet: ( keys: Array, callback: ( - errors: ?Array<{|message: string|}>, + errors: ?Array<{message: string, key?: string}>, kvPairs: ?Array>, ) => void, ) => void; +multiSet: ( kvPairs: Array>, - callback: (errors: ?Array<{|message: string|}>) => void, + callback: (errors: ?Array<{message: string, key?: string}>) => void, ) => void; +multiMerge: ( kvPairs: Array>, - callback: (errors: ?Array<{|message: string|}>) => void, + callback: (errors: ?Array<{message: string, key?: string}>) => void, ) => void; +multiRemove: ( keys: Array, - callback: (errors: ?Array<{|message: string|}>) => void, + callback: (errors: ?Array<{message: string, key?: string}>) => void, ) => void; - +clear: (callback: (error: {|message: string|}) => void) => void; + +clear: (callback: (error: {message: string, key?: string}) => void) => void; +getAllKeys: ( - callback: (error: ?{|message: string|}, allKeys: ?Array) => void, + callback: ( + error: ?{message: string, key?: string}, + allKeys: ?Array, + ) => void, ) => void; } diff --git a/Libraries/Storage/NativeAsyncSQLiteDBStorage.js b/Libraries/Storage/NativeAsyncSQLiteDBStorage.js index 6009a98d14a..d3431643370 100644 --- a/Libraries/Storage/NativeAsyncSQLiteDBStorage.js +++ b/Libraries/Storage/NativeAsyncSQLiteDBStorage.js @@ -14,29 +14,32 @@ import type {TurboModule} from '../TurboModule/RCTExport'; import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry'; export interface Spec extends TurboModule { - +getConstants: () => {||}; + +getConstants: () => {}; +multiGet: ( keys: Array, callback: ( - errors: ?Array<{|message: string|}>, + errors: ?Array<{message: string, key?: string}>, kvPairs: ?Array>, ) => void, ) => void; +multiSet: ( kvPairs: Array>, - callback: (errors: ?Array<{|message: string|}>) => void, + callback: (errors: ?Array<{message: string, key?: string}>) => void, ) => void; +multiMerge: ( kvPairs: Array>, - callback: (errors: ?Array<{|message: string|}>) => void, + callback: (errors: ?Array<{message: string, key?: string}>) => void, ) => void; +multiRemove: ( keys: Array, - callback: (errors: ?Array<{|message: string|}>) => void, + callback: (errors: ?Array<{message: string, key?: string}>) => void, ) => void; - +clear: (callback: (error: {|message: string|}) => void) => void; + +clear: (callback: (error: {message: string, key?: string}) => void) => void; +getAllKeys: ( - callback: (error: ?{|message: string|}, allKeys: ?Array) => void, + callback: ( + error: ?{message: string, key?: string}, + allKeys: ?Array, + ) => void, ) => void; } diff --git a/packages/rn-tester/js/utils/RNTesterStatePersister.js b/packages/rn-tester/js/utils/RNTesterStatePersister.js index 8130a69c38b..cc5084c6d0c 100644 --- a/packages/rn-tester/js/utils/RNTesterStatePersister.js +++ b/packages/rn-tester/js/utils/RNTesterStatePersister.js @@ -59,7 +59,11 @@ function createContainer( _passSetState = (stateLamda: (state: State) => State): void => { this.setState(state => { const value = stateLamda(state.value); - AsyncStorage.setItem(this._cacheKey, JSON.stringify(value)); + AsyncStorage.setItem( + this._cacheKey, + // $FlowFixMe[incompatible-call] Error surfaced when typing AsyncStorage + JSON.stringify(value), + ); return {value}; }); };