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

Defer initialization and wait for defaultKeyStates #91

Merged
merged 7 commits into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 50 additions & 18 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Str from 'expensify-common/lib/str';
import lodashMerge from 'lodash/merge';
import {registerLogger, logInfo, logAlert} from './Logger';
import cache from './OnyxCache';
import createDeferredTask from './createDeferredTask';

// Keeps track of the last connectionID that was used so we can keep incrementing it
let lastConnectionID = 0;
Expand All @@ -28,6 +29,9 @@ const evictionBlocklist = {};
// Optional user-provided key value states set when Onyx initializes or clears
let defaultKeyStates = {};

// Connections can be made before `Onyx.init`. They would wait for this task before resolving
const deferredInitTask = createDeferredTask();

/**
* Get some data from the store
*
Expand Down Expand Up @@ -199,9 +203,11 @@ function addToEvictionBlockList(key, connectionID) {
* the recently accessed list when initializing the app. This
* enables keys that have not recently been accessed to be
* removed.
*
* @returns {Promise}
*/
function addAllSafeEvictionKeysToRecentlyAccessedList() {
getAllKeys()
return getAllKeys()
kidroca marked this conversation as resolved.
Show resolved Hide resolved
.then((keys) => {
_.each(evictionAllowList, (safeEvictionKey) => {
_.each(keys, (key) => {
Expand Down Expand Up @@ -359,18 +365,21 @@ function connect(mapping) {
return connectionID;
}

// Check to see if this key is flagged as a safe eviction key and add it to the recentlyAccessedKeys list
if (mapping.withOnyxInstance && !isCollectionKey(mapping.key) && isSafeEvictionKey(mapping.key)) {
// All React components subscribing to a key flagged as a safe eviction
// key must implement the canEvict property.
if (_.isUndefined(mapping.canEvict)) {
// eslint-disable-next-line max-len
throw new Error(`Cannot subscribe to safe eviction key '${mapping.key}' without providing a canEvict value.`);
}
addLastAccessedKey(mapping.key);
}

getAllKeys()
// Commit connection only after init passes
deferredInitTask.promise
.then(() => {
// Check to see if this key is flagged as a safe eviction key and add it to the recentlyAccessedKeys list
if (mapping.withOnyxInstance && !isCollectionKey(mapping.key) && isSafeEvictionKey(mapping.key)) {
// All React components subscribing to a key flagged as a safe eviction
// key must implement the canEvict property.
if (_.isUndefined(mapping.canEvict)) {
// eslint-disable-next-line max-len
throw new Error(`Cannot subscribe to safe eviction key '${mapping.key}' without providing a canEvict value.`);
}
addLastAccessedKey(mapping.key);
}
})
.then(getAllKeys)
.then((keys) => {
// Find all the keys matched by the config key
const matchingKeys = _.filter(keys, key => isKeyMatch(mapping.key, key));
Expand Down Expand Up @@ -408,6 +417,11 @@ function connect(mapping) {
* @param {string} key
*/
function cleanCache(key) {
// Don't remove default keys from cache, they don't take much memory and are accessed frequently
if (_.has(defaultKeyStates, key)) {
return;
}

const hasRemainingConnections = _.some(callbackToStateMapping, {key});

// When the key is still used in other places don't remove it from cache
Expand Down Expand Up @@ -624,9 +638,21 @@ function merge(key, val) {

/**
* Merge user provided default key value pairs.
*
* @returns {Promise}
*/
function initializeWithDefaultKeyStates() {
_.each(defaultKeyStates, (state, key) => merge(key, state));
return AsyncStorage.multiGet(_.keys(defaultKeyStates))
Copy link
Contributor

Choose a reason for hiding this comment

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

Just caught this. But do the "after" metrics in the description include this time at all?

Copy link
Contributor Author

@kidroca kidroca Jul 29, 2021

Choose a reason for hiding this comment

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

Yes, because we decorate and track this method here:
https://github.com/kidroca/react-native-onyx/blob/9549e4057abaf2a8f96bad1f255acd1c0b002cd2/lib/Onyx.js#L809
It's tracked under Onyx:defaults and adds time against "total time"

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok cool. Thanks.

.then((pairs) => {
const asObject = _.chain(pairs)
.map(([key, val]) => [key, val && JSON.parse(val)])
.object()
.value();

const merged = lodashMerge(asObject, defaultKeyStates);
cache.merge(merged);
_.each(merged, (val, key) => keyChanged(key, val));
kidroca marked this conversation as resolved.
Show resolved Hide resolved
});
}

/**
Expand Down Expand Up @@ -701,7 +727,9 @@ function mergeCollection(collectionKey, collection) {
/**
* Initialize the store with actions and listening for storage events
*
* @param {Object} [options]
* @param {Object} options
* @param {Object} [options.keys]
* @param {Object} [options.initialKeyStates]
* @param {String[]} [options.safeEvictionKeys] This is an array of keys
* (individual or collection patterns) that when provided to Onyx are flagged
* as "safe" for removal. Any components subscribing to these keys must also
Expand Down Expand Up @@ -732,10 +760,13 @@ function init({

// Let Onyx know about which keys are safe to evict
evictionAllowList = safeEvictionKeys;
addAllSafeEvictionKeysToRecentlyAccessedList();

// Initialize all of our keys with data provided
initializeWithDefaultKeyStates();
// Initialize all of our keys with data provided then give green light to any pending connections
Promise.all([
addAllSafeEvictionKeysToRecentlyAccessedList(),
initializeWithDefaultKeyStates()
])
.then(deferredInitTask.resolve);

// Update any key whose value changes in storage
registerStorageEventListener((key, newValue) => {
Expand Down Expand Up @@ -775,6 +806,7 @@ function applyDecorators() {
merge = decorate.decorateWithMetrics(merge, 'Onyx:merge');
mergeCollection = decorate.decorateWithMetrics(mergeCollection, 'Onyx:mergeCollection');
getAllKeys = decorate.decorateWithMetrics(getAllKeys, 'Onyx:getAllKeys');
initializeWithDefaultKeyStates = decorate.decorateWithMetrics(initializeWithDefaultKeyStates, 'Onyx:defaults');
kidroca marked this conversation as resolved.
Show resolved Hide resolved
/* eslint-enable */

// Re-expose decorated methods
Expand Down
16 changes: 16 additions & 0 deletions lib/createDeferredTask.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Create a deferred task that can be resolved when we call `resolve()`
* The returned promise will complete when we call `resolve`
* Useful when we want to wait for a tasks that is resolved from an external action
*
* @template T
* @returns {{ resolve: function(*), promise: Promise<T|void> }}
*/
export default function createDeferredTask() {
const deferred = {};
deferred.promise = new Promise((res) => {
deferred.resolve = res;
});

return deferred;
}