Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TS migration] Migrate Onyx public methods to TS #512

Merged
merged 23 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ed93e94
Remove memory only keys logic
blazejkustra Mar 11, 2024
396e926
2.0.22
OSBotify Mar 12, 2024
db5c103
Split Onyx.js into two files
blazejkustra Mar 12, 2024
62476eb
Merge branch 'ts/onyx-js-preparation' into ts/Onyx-public-methods
blazejkustra Mar 13, 2024
c924a56
Import from OnyxUtils
blazejkustra Mar 13, 2024
7a65a45
Move types to types.ts, reorder functions
blazejkustra Mar 13, 2024
430825e
Change logSetStateCall argument type
blazejkustra Mar 13, 2024
31d0a78
Export OnyxUtils as default export
blazejkustra Mar 13, 2024
4437e63
Move part of Onyx.init code to OnyxUtils
blazejkustra Mar 13, 2024
f2cb2ee
Merge branch 'ts/onyx-js-preparation' into ts/Onyx-public-methods
blazejkustra Mar 14, 2024
b747b18
Fix public methods declarations
blazejkustra Mar 14, 2024
261021d
Merge branch 'main' into ts/Onyx-public-methods
blazejkustra Mar 15, 2024
566baa4
Fix type errors
blazejkustra Mar 15, 2024
d4a2003
Adjust return types
blazejkustra Mar 15, 2024
b4a6af7
Small fixes after initial review
blazejkustra Mar 19, 2024
bb10ff9
Fix imports in withOnyx
blazejkustra Mar 19, 2024
83b9194
Fix tests, convert Set to array
blazejkustra Mar 19, 2024
d479c06
Fix issues with getAllKeys
blazejkustra Mar 19, 2024
8c9b867
Fix comment, from OnyxUtils to Omnyx
blazejkustra Mar 19, 2024
c6c7a84
Fix a bug that default values weren't set properly
blazejkustra Mar 19, 2024
0e9acd3
Bring back JSDoc for conect()
blazejkustra Mar 20, 2024
9dc8ed2
Add getters to OnyxUtils
blazejkustra Mar 20, 2024
78b00ca
Address minor comments
blazejkustra Mar 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,16 @@ function init({
* @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the
* component
* @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object
blazejkustra marked this conversation as resolved.
Show resolved Hide resolved
* @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data.
* The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive
* performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally
* cause the component to re-render (and that can be expensive from a performance standpoint).
* @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx().
* If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB.
* Note that it will not cause the component to have the loading prop set to true. |
blazejkustra marked this conversation as resolved.
Show resolved Hide resolved
* @returns an ID to use when calling disconnect
*/
function connect<TKey extends OnyxKey>(options: ConnectOptions<TKey>): number {
const mapping = options as unknown as Mapping<OnyxKey>;

function connect<TKey extends OnyxKey>(mapping: ConnectOptions<TKey>): number {
const connectionID = lastConnectionID++;
OnyxUtils.callbackToStateMapping[connectionID] = mapping;
OnyxUtils.callbackToStateMapping[connectionID].connectionID = connectionID;
Expand All @@ -100,15 +105,15 @@ function connect<TKey extends OnyxKey>(options: ConnectOptions<TKey>): number {
// If the mapping is connected to an onyx key that is not a collection
// we can skip the call to getAllKeys() and return an array with a single item
if (Boolean(mapping.key) && typeof mapping.key === 'string' && !mapping.key.endsWith('_') && cache.storageKeys.has(mapping.key)) {
return [mapping.key];
return new Set([mapping.key]);
blazejkustra marked this conversation as resolved.
Show resolved Hide resolved
}
return OnyxUtils.getAllKeys();
})
.then((keys) => {
// We search all the keys in storage to see if any are a "match" for the subscriber we are connecting so that we
// can send data back to the subscriber. Note that multiple keys can match as a subscriber could either be
// subscribed to a "collection key" or a single key.
const matchingKeys = keys.filter((key) => OnyxUtils.isKeyMatch(mapping.key, key));
const matchingKeys = Array.from(keys).filter((key) => OnyxUtils.isKeyMatch(mapping.key, key));

// If the key being connected to does not exist we initialize the value with null. For subscribers that connected
// directly via connect() they will simply get a null value sent to them without any information about which key matched
Expand Down Expand Up @@ -387,8 +392,8 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TK
return true;
});

const existingKeys = keys.filter((key) => persistedKeys.includes(key));
const newKeys = keys.filter((key) => !persistedKeys.includes(key));
const existingKeys = keys.filter((key) => persistedKeys.has(key));
const newKeys = keys.filter((key) => !persistedKeys.has(key));

const existingKeyCollection = existingKeys.reduce((obj: NullableKeyValueMapping, key) => {
// eslint-disable-next-line no-param-reassign
Expand Down
2 changes: 1 addition & 1 deletion lib/OnyxUtils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ declare function get(key: OnyxKey): Promise<OnyxValue<OnyxKey>>;
/**
* Returns current key names stored in persisted storage
*/
declare function getAllKeys(): Promise<OnyxKey[]>;
declare function getAllKeys(): Promise<Set<string>>;

/**
* Checks to see if the a subscriber's supplied key
Expand Down
4 changes: 2 additions & 2 deletions lib/OnyxUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ let evictionAllowList = [];
const evictionBlocklist = {};

// Optional user-provided key value states set when Onyx initializes or clears
let defaultKeyStates = {};
const defaultKeyStates = {};

let batchUpdatesPromise = null;
let batchUpdatesQueue = [];
Expand All @@ -67,7 +67,7 @@ function initStoreValues(keys, initialKeyStates, safeEvictionKeys) {
);

// Set our default key states to use when initializing and clearing Onyx data
defaultKeyStates = initialKeyStates;
_.assign(defaultKeyStates, initialKeyStates);

DevTools.initState(initialKeyStates);

Expand Down
2 changes: 1 addition & 1 deletion lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ type BaseConnectOptions = {
type ConnectOptions<TKey extends OnyxKey> = BaseConnectOptions &
(
| {
key: TKey extends CollectionKey ? TKey : never;
key: TKey extends CollectionKeyBase ? TKey : never;
callback?: (value: OnyxCollection<KeyValueMapping[TKey]>) => void;
waitForCollectionCallback: true;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/useOnyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
}

if (!OnyxUtils.isSafeEvictionKey(key)) {
throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to OnyxUtils.init({safeEvictionKeys: []}).`);
throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`);
}

if (options.canEvict) {
Expand Down
11 changes: 6 additions & 5 deletions lib/withOnyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import _ from 'underscore';
import Onyx from './Onyx';
import * as Str from './Str';
import utils from './utils';
import OnyxUtils from './OnyxUtils';

// This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check
// if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results.
Expand Down Expand Up @@ -58,7 +59,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) {
mapOnyxToState,
(resultObj, mapping, propertyName) => {
const key = Str.result(mapping.key, props);
let value = Onyx.tryGetCachedValue(key, mapping);
let value = OnyxUtils.tryGetCachedValue(key, mapping);
if (!value && mapping.initialValue) {
value = mapping.initialValue;
}
Expand All @@ -74,7 +75,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) {
* In reality, Onyx.merge() will only update the subscriber after all merges have been batched and the previous value is retrieved via a get() (returns a promise).
* So, we won't use the cache optimization here as it will lead us to arbitrarily defer various actions in the application code.
*/
if ((value !== undefined && !Onyx.hasPendingMergeForKey(key)) || mapping.allowStaleData) {
if ((value !== undefined && !OnyxUtils.hasPendingMergeForKey(key)) || mapping.allowStaleData) {
// eslint-disable-next-line no-param-reassign
resultObj[propertyName] = value;
}
Expand Down Expand Up @@ -256,14 +257,14 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) {
const canEvict = Str.result(mapping.canEvict, this.props);
const key = Str.result(mapping.key, this.props);

if (!Onyx.isSafeEvictionKey(key)) {
if (!OnyxUtils.isSafeEvictionKey(key)) {
throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`);
}

if (canEvict) {
Onyx.removeFromEvictionBlockList(key, mapping.connectionID);
OnyxUtils.removeFromEvictionBlockList(key, mapping.connectionID);
} else {
Onyx.addToEvictionBlockList(key, mapping.connectionID);
OnyxUtils.addToEvictionBlockList(key, mapping.connectionID);
}
});
}
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/onyxTest.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'underscore';
import Onyx from '../../lib';
import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
import OnyxUtils from '../../lib/OnyxUtils';

const ONYX_KEYS = {
TEST_KEY: 'test',
Expand Down Expand Up @@ -38,7 +39,7 @@ describe('Onyx', () => {

it('should remove key value from OnyxCache/Storage when set is called with null value', () =>
Onyx.set(ONYX_KEYS.OTHER_TEST, 42)
.then(() => Onyx.getAllKeys())
.then(() => OnyxUtils.getAllKeys())
.then((keys) => {
expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true);
return Onyx.set(ONYX_KEYS.OTHER_TEST, null);
Expand All @@ -48,7 +49,7 @@ describe('Onyx', () => {
expect(cache.getAllKeys().size).toBe(0);

// When cache keys length is 0, we fetch the keys from storage.
return Onyx.getAllKeys();
return OnyxUtils.getAllKeys();
})
.then((keys) => {
expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false);
Expand Down
Loading