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

feat: add integration with Redux DevTools #289

Merged
merged 6 commits into from
Sep 6, 2023
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
1 change: 1 addition & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ Initialize the store with actions and listening for storage events
| [options.captureMetrics] | <code>Boolean</code> | | Enables Onyx benchmarking and exposes the get/print/reset functions |
| [options.shouldSyncMultipleInstances] | <code>Boolean</code> | | Auto synchronize storage events between multiple instances of Onyx running in different tabs/windows. Defaults to true for platforms that support local storage (web/desktop) |
| [options.debugSetState] | <code>Boolean</code> | | Enables debugging setState() calls to connected components. |
| [options.enableDevTools] | <code>Boolean</code> | | Enables debugging using Redux DevTools extension. |

**Example**
```js
Expand Down
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,18 @@ Sample output of `Onyx.printMetrics()`

# Debug mode

## Using debugSetState

It can be useful to log why Onyx is calling `setState()` on a particular React component so that we can understand which key changed, what changed about the value, and the connected component that ultimately rendered as a result. When used correctly this can help isolate problem areas and unnecessary renders in the code. To enable this feature, pass `debugSetState: true` to the config and grep JS console logs for `[Onyx-Debug]`.

## Using Redux DevTools extension

It can be useful to check the order of writes to the storage and it's state at a specific point in time.

First, install Redux DevTools through your favorite browser ([Edge](https://microsoftedge.microsoft.com/addons/detail/redux-devtools/nnkgneoiohoecpdiaponcejilbhhikei), [Chrome](https://chrome.google.com/webstore/detail/redux-devtools/lmhkpmbekcpmknklioeibfkpmmfibljd), [Firefox](https://addons.mozilla.org/en-US/firefox/addon/reduxdevtools/))

Then, you can enable this type of debugging by passing `enableDevTools: true` to `Onyx.init({...})`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I kinda wonder if this should be enabled by default on dev?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AndrewGable I'm not sure if it won't be impactful on performance 😅


# Development

`react-native` bundles source using the `metro` bundler. `metro` does not follow symlinks, so we can't use `npm link` to
Expand Down
49 changes: 49 additions & 0 deletions lib/DevTools.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {connectViaExtension} from 'remotedev';
import _ from 'underscore';

class DevTools {
/**
* @callback onStateChange
* @param {object} state
*/
/**
* Creates an instance of DevTools, with an internal state that mirrors the storage.
*
* @param {object} initialState - initial state of the storage
* @param {onStateChange} onStateChange - callback which is triggered when we timetravel to a different registered action
*/
constructor(initialState = {}) {
this.state = initialState;
this.remotedev = connectViaExtension();
this.remotedev.init(this.state);
}

/**
* Registers an action that updated the current state of the storage
*
* @param {string} type - name of the action
* @param {any} payload - data written to the storage
* @param {object} stateChanges - partial state that got updated after the changes
*/
registerAction(type, payload = undefined, stateChanges = {}) {
const newState = {
...this.state,
...stateChanges,
};

this.remotedev.send({type, payload}, newState);
this.state = newState;
}

/**
* This clears the internal state of the DevTools, preserving the keys not included in `keyToBeRemoved`
*
* @param {string[]} keysToBeRemoved
*/
clearState(keysToBeRemoved = []) {
const pairs = _.map(keysToBeRemoved, key => [key, undefined]);
this.registerAction('CLEAR', undefined, _.object((pairs)));
}
}

export default DevTools;
55 changes: 48 additions & 7 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import createDeferredTask from './createDeferredTask';
import fastMerge from './fastMerge';
import * as PerformanceUtils from './metrics/PerformanceUtils';
import Storage from './storage';
import DevTools from './DevTools';

// Method constants
const METHOD = {
Expand Down Expand Up @@ -37,6 +38,11 @@ let recentlyAccessedKeys = [];
// whatever appears in this list it will NEVER be a candidate for eviction.
let evictionAllowList = [];

let devTools = {
registerAction: () => {},
clearState: () => {},
};

// Holds a map of keys and connectionID arrays whose keys will never be automatically evicted as
// long as we have at least one subscriber that returns false for the canEvict property.
const evictionBlocklist = {};
Expand Down Expand Up @@ -842,7 +848,10 @@ function notifyCollectionSubscribersOnNextTick(key, value) {
function remove(key) {
cache.drop(key);
notifySubscribersOnNextTick(key, null);
return Storage.removeItem(key);
return Storage.removeItem(key).then((result) => {
devTools.registerAction(`REMOVE/${key.toUpperCase()}`, undefined, {[key]: undefined});
return result;
});
}

/**
Expand Down Expand Up @@ -966,6 +975,7 @@ function set(key, value) {
const hasChanged = cache.hasValueChanged(key, valueWithNullRemoved);

// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
broadcastUpdate(key, value, 'set');
broadcastUpdate(key, valueWithNullRemoved, hasChanged, 'set');

// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
Expand All @@ -974,6 +984,10 @@ function set(key, value) {
}

return Storage.setItem(key, valueWithNullRemoved)
.then((result) => {
devTools.registerAction(`SET/${key.toUpperCase()}`, valueWithNullRemoved, {[key]: valueWithNullRemoved});
return result;
})
.catch(error => evictStorageAndRetry(error, set, key, valueWithNullRemoved));
}

Expand Down Expand Up @@ -1004,6 +1018,11 @@ function multiSet(data) {
// Update cache and optimistically inform subscribers on the next tick
cache.set(key, val);
notifySubscribersOnNextTick(key, val);
if (_.isNull(val)) {
devTools.registerAction(`REMOVE/${key.toUpperCase()}`, val, {[key]: undefined});
} else {
devTools.registerAction(`SET/${key.toUpperCase()}`, val, {[key]: val});
}
});

return Storage.multiSet(keyValuePairs)
Expand Down Expand Up @@ -1098,7 +1117,10 @@ function merge(key, changes) {
return Promise.resolve();
}

return Storage.mergeItem(key, batchedChanges, modifiedData);
return Storage.mergeItem(key, batchedChanges, modifiedData).then((results) => {
devTools.registerAction(`MERGE/${key.toUpperCase()}`, modifiedData, {[key]: modifiedData});
return results;
});
} catch (error) {
Logger.logAlert(`An error occurred while applying merge for key: ${key}, Error: ${error}`);
}
Expand All @@ -1110,16 +1132,20 @@ function merge(key, changes) {
/**
* Merge user provided default key value pairs.
* @private
* @param {boolean} enableDevTools
* @returns {Promise}
*/
function initializeWithDefaultKeyStates() {
function initializeWithDefaultKeyStates(enableDevTools = false) {
return Storage.multiGet(_.keys(defaultKeyStates))
.then((pairs) => {
const asObject = _.object(pairs);

const merged = fastMerge(asObject, defaultKeyStates);
cache.merge(merged);
_.each(merged, (val, key) => keyChanged(key, val));
if (enableDevTools) {
devTools = new DevTools(merged);
}
});
}

Expand Down Expand Up @@ -1167,7 +1193,7 @@ function clear(keysToPreserve = []) {
// since collection key subscribers need to be updated differently
if (!isKeyToPreserve) {
const oldValue = cache.getValue(key);
const newValue = _.get(defaultKeyStates, key, null);
const newValue = _.get(defaultKeyStates, key, undefined);
if (newValue !== oldValue) {
cache.set(key, newValue);
const collectionKey = key.substring(0, key.indexOf('_') + 1);
Expand Down Expand Up @@ -1198,11 +1224,15 @@ function clear(keysToPreserve = []) {
notifyCollectionSubscribersOnNextTick(key, value);
});

const defaultKeyValuePairs = _.pairs(_.omit(defaultKeyStates, keysToPreserve));
const defaultKeyValueState = _.omit(defaultKeyStates, keysToPreserve);
const defaultKeyValuePairs = _.pairs(defaultKeyValueState);

// Remove only the items that we want cleared from storage, and reset others to default
_.each(keysToBeClearedFromStorage, key => cache.drop(key));
return Storage.removeItems(keysToBeClearedFromStorage).then(() => Storage.multiSet(defaultKeyValuePairs));
return Storage.removeItems(keysToBeClearedFromStorage).then(() => {
devTools.clearState(keysToBeClearedFromStorage);
return Storage.multiSet(defaultKeyValuePairs);
});
});
}

Expand Down Expand Up @@ -1276,6 +1306,11 @@ function mergeCollection(collectionKey, collection) {
Promise.all(_.map(existingKeys, get)).then(() => {
cache.merge(collection);
keysChanged(collectionKey, collection);
if (_.isNull(collection)) {
devTools.registerAction(`REMOVE/${collectionKey.toUpperCase()}`, collection, {[collectionKey]: undefined});
} else {
devTools.registerAction(`SET/${collectionKey.toUpperCase()}`, collection, {[collectionKey]: collection});
}
});

return Promise.all(promises)
Expand Down Expand Up @@ -1377,6 +1412,7 @@ function init({
captureMetrics = false,
shouldSyncMultipleInstances = Boolean(global.localStorage),
debugSetState = false,
enableDevTools = false,
} = {}) {
if (captureMetrics) {
// The code here is only bundled and applied when the captureMetrics is set
Expand Down Expand Up @@ -1404,14 +1440,19 @@ function init({
// Initialize all of our keys with data provided then give green light to any pending connections
Promise.all([
addAllSafeEvictionKeysToRecentlyAccessedList(),
initializeWithDefaultKeyStates(),
initializeWithDefaultKeyStates(enableDevTools),
])
.then(deferredInitTask.resolve);

if (shouldSyncMultipleInstances && _.isFunction(Storage.keepInstancesSync)) {
Storage.keepInstancesSync((key, value) => {
cache.set(key, value);
keyChanged(key, value);
if (_.isNull(value)) {
devTools.registerAction(`REMOVE/${key.toUpperCase()}`, value, {[key]: undefined});
} else {
devTools.registerAction(`SET/${key.toUpperCase()}`, value, {[key]: value});
}
});
}
}
Expand Down
Loading
Loading