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.

Sign up for GitHub

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

Jump to bottom

Kidroca/onyx cache cleanup #79

Merged
merged 5 commits into from
Jun 8, 2021
Merged

Kidroca/onyx cache cleanup #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7a3c0a9
fix: update cache on storage events
kidroca Jun 8, 2021
8b32b16
docs: remove todo comments
kidroca Jun 8, 2021
05779f7
docs: fix typos
kidroca Jun 8, 2021
280d066
test: update `AsyncStorageMock` require calls
kidroca Jun 8, 2021
5d83843
fix: OnyxCache.merge does not update storage keys
kidroca Jun 8, 2021
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
5 changes: 4 additions & 1 deletion lib/Onyx.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,10 @@ function init({
initializeWithDefaultKeyStates();

// Update any key whose value changes in storage
registerStorageEventListener((key, newValue) => keyChanged(key, newValue));
registerStorageEventListener((key, newValue) => {
cache.set(key, newValue);
keyChanged(key, newValue);
});
}

const Onyx = {
Expand Down
6 changes: 5 additions & 1 deletion lib/OnyxCache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,15 @@ class OnyxCache {
}

/**
* Deep merge data to cache, any non existing keys will be crated
* Deep merge data to cache, any non existing keys will be created
* @param {Record<string, *>} data - a map of (cache) key - values
*/
merge(data) {
this.storageMap = lodashMerge({}, this.storageMap, data);

const storageKeys = this.getAllKeys();
const mergedKeys = _.keys(data);
this.storageKeys = new Set([...storageKeys, ...mergedKeys]);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/decorateWithMetrics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function decorateWithMetrics(func, alias = func.name) {

/*
* Then handlers added here are not affecting the original promise
* They crate a separate chain that's not exposed (returned) to the original caller
* They create a separate chain that's not exposed (returned) to the original caller
* */
originalPromise
.finally(() => {
Expand Down
32 changes: 18 additions & 14 deletions tests/unit/onyxCacheTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,22 @@ describe('Onyx', () => {
expect(cache.hasCacheForKey('mockKey')).toBe(true);
expect(cache.getValue('mockKey')).toEqual({ID: 5});
});

it('Should update storageKeys when new keys are created', () => {
// GIVEN cache with some items
cache.set('mockKey', {value: 'mockValue'});
cache.set('mockKey2', {other: 'otherMockValue', mock: 'mock', items: [3, 4, 5]});

// WHEN merge is called with existing key value pairs
cache.merge({
mockKey: {mockItems: []},
mockKey3: {ID: 3},
mockKey4: {ID: 4},
});

// THEN getAllStorage keys should return updated storage keys
expect(cache.getAllKeys()).toEqual(['mockKey', 'mockKey2', 'mockKey3', 'mockKey4']);
});
});

describe('hasPendingTask', () => {
Expand Down Expand Up @@ -377,6 +393,7 @@ describe('Onyx', () => {
describe('Onyx with Cache', () => {
let Onyx;
let withOnyx;
let AsyncStorageMock;

/** @type OnyxCache */
let cache;
Expand All @@ -393,6 +410,7 @@ describe('Onyx', () => {
const OnyxModule = require('../../index');
Onyx = OnyxModule.default;
withOnyx = OnyxModule.withOnyx;
AsyncStorageMock = require('@react-native-community/async-storage').default;
cache = require('../../lib/OnyxCache').default;

Onyx.init({
Expand All @@ -413,8 +431,6 @@ describe('Onyx', () => {
});

it('Expect a single call to getItem when multiple components use the same key', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN a component connected to Onyx
const TestComponentWithOnyx = withOnyx({
text: {
Expand Down Expand Up @@ -444,8 +460,6 @@ describe('Onyx', () => {
});

it('Expect a single call to getAllKeys when multiple components use the same key', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN a component connected to Onyx
const TestComponentWithOnyx = withOnyx({
text: {
Expand Down Expand Up @@ -476,8 +490,6 @@ describe('Onyx', () => {
});

it('Expect multiple calls to getItem when no existing component is using a key', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN a component connected to Onyx
const TestComponentWithOnyx = withOnyx({
text: {
Expand Down Expand Up @@ -509,8 +521,6 @@ describe('Onyx', () => {
});

it('Expect multiple calls to getItem when multiple keys are used', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN two component
const TestComponentWithOnyx = withOnyx({
testObject: {
Expand Down Expand Up @@ -550,8 +560,6 @@ describe('Onyx', () => {
});

it('Expect a single call to getItem when at least one component is still subscribed to a key', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN a component connected to Onyx
const TestComponentWithOnyx = withOnyx({
text: {
Expand Down Expand Up @@ -589,8 +597,6 @@ describe('Onyx', () => {
});

it('Should remove collection items from cache when collection is disconnected', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN a component subscribing to a collection
const TestComponentWithOnyx = withOnyx({
collections: {
Expand Down Expand Up @@ -638,8 +644,6 @@ describe('Onyx', () => {
});

it('Should not remove item from cache when it still used in a collection', () => {
const AsyncStorageMock = require('@react-native-community/async-storage/jest/async-storage-mock');

// GIVEN component that uses a collection and a component that uses a collection item
const COLLECTION_ITEM_KEY = `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}10`;
const TestComponentWithOnyx = withOnyx({
Expand Down
4 changes: 0 additions & 4 deletions tests/unit/withOnyxTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ describe('withOnyx', () => {
});

it('should update withOnyx subscriber multiple times when merge is used', () => {
// Todo: ViewWithCollections does not expect a `text` prop
const TestComponentWithOnyx = withOnyx({
text: {
key: ONYX_KEYS.COLLECTION.TEST_KEY,
Expand All @@ -62,7 +61,6 @@ describe('withOnyx', () => {
});

it('should update withOnyx subscriber just once when mergeCollection is used', () => {
// Todo: ViewWithCollections does not expect a `text` prop
const TestComponentWithOnyx = withOnyx({
text: {
key: ONYX_KEYS.COLLECTION.TEST_KEY,
Expand All @@ -79,7 +77,6 @@ describe('withOnyx', () => {
});

it('should update withOnyx subscribing to individual key if mergeCollection is used', () => {
// Todo: ViewWithCollections does not expect a `text` prop
const collectionItemID = 1;
const TestComponentWithOnyx = withOnyx({
text: {
Expand All @@ -97,7 +94,6 @@ describe('withOnyx', () => {
});

it('should update withOnyx subscribing to individual key with merged value if mergeCollection is used', () => {
// Todo: ViewWithCollections does not expect a `text` prop
const collectionItemID = 4;
const TestComponentWithOnyx = withOnyx({
text: {
Expand Down