From 94372ef96afc162f756ef638a53ff151f7a45f05 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 27 Oct 2023 11:46:58 -0600 Subject: [PATCH] Revert "Merge pull request #401 from janicduplessis/@janic/cache-fixes" This reverts commit a8c01150450663052cf1538ea68a2873fdcedddb, reversing changes made to 96720736acb556fdf9d4728d08ea6e0af509e861. --- lib/Onyx.js | 18 ++-- tests/components/ViewWithCollections.js | 4 - tests/components/ViewWithText.js | 5 +- tests/unit/withOnyxTest.js | 109 ------------------------ 4 files changed, 7 insertions(+), 129 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 7bb3f4e7..a940afc9 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -235,14 +235,8 @@ function tryGetCachedValue(key, mapping = {}) { let val = cache.getValue(key); if (isCollectionKey(key)) { - const allCacheKeys = cache.getAllKeys(); - - // It is possible we haven't loaded all keys yet so we do not know if the - // collection actually exists. - if (allCacheKeys.length === 0) { - return; - } - const matchingKeys = _.filter(allCacheKeys, k => k.startsWith(key)); + const allKeys = cache.getAllKeys(); + const matchingKeys = _.filter(allKeys, k => k.startsWith(key)); const values = _.reduce(matchingKeys, (finalObject, matchedKey) => { const cachedValue = cache.getValue(matchedKey); if (cachedValue) { @@ -252,7 +246,9 @@ function tryGetCachedValue(key, mapping = {}) { } return finalObject; }, {}); - + if (_.isEmpty(values)) { + return; + } val = values; } @@ -830,10 +826,6 @@ function connect(mapping) { // since there are none matched. In withOnyx() we wait for all connected keys to return a value before rendering the child // component. This null value will be filtered out so that the connected component can utilize defaultProps. if (matchingKeys.length === 0) { - if (mapping.key && !isCollectionKey(mapping.key)) { - cache.set(mapping.key, null); - } - // Here we cannot use batching because the null value is expected to be set immediately for default props // or they will be undefined. sendDataToConnection(mapping, null, undefined, false); diff --git a/tests/components/ViewWithCollections.js b/tests/components/ViewWithCollections.js index aa05d0b9..ecf380ef 100644 --- a/tests/components/ViewWithCollections.js +++ b/tests/components/ViewWithCollections.js @@ -29,10 +29,6 @@ const ViewWithCollections = React.forwardRef((props, ref) => { props.onRender(props); - if (_.size(props.collections) === 0) { - return empty; - } - return ( {_.map(props.collections, (collection, i) => ( diff --git a/tests/components/ViewWithText.js b/tests/components/ViewWithText.js index 0a872077..e172f295 100644 --- a/tests/components/ViewWithText.js +++ b/tests/components/ViewWithText.js @@ -4,13 +4,12 @@ import PropTypes from 'prop-types'; import {View, Text} from 'react-native'; const propTypes = { - text: PropTypes.string, + text: PropTypes.string.isRequired, onRender: PropTypes.func, }; const defaultProps = { onRender: () => {}, - text: null, }; function ViewWithText(props) { @@ -18,7 +17,7 @@ function ViewWithText(props) { return ( - {props.text || 'null'} + {props.text} ); } diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 56207dbd..82fc544b 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -6,7 +6,6 @@ import ViewWithText from '../components/ViewWithText'; import ViewWithCollections from '../components/ViewWithCollections'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import ViewWithObject from '../components/ViewWithObject'; -import StorageMock from '../../lib/storage'; const ONYX_KEYS = { TEST_KEY: 'test', @@ -523,112 +522,4 @@ describe('withOnyxTest', () => { expect(onRender.mock.calls[3][0].simple).toBe('long_string'); }); }); - - it('should render immediately when a onyx component is mounted a 2nd time', () => { - const TestComponentWithOnyx = withOnyx({ - text: { - key: ONYX_KEYS.TEST_KEY, - }, - })(ViewWithText); - const onRender = jest.fn(); - let renderResult; - - // Set the value in storage, but not in cache. - return StorageMock.setItem(ONYX_KEYS.TEST_KEY, 'test1') - .then(() => { - renderResult = render(); - - // The component should not render initially since we have no data in cache. - // Use `waitForPromisesToResolve` before making the assertions and make sure - // onRender was not called. - expect(onRender).not.toHaveBeenCalled(); - return waitForPromisesToResolve(); - }) - .then(waitForPromisesToResolve) - .then(() => { - let textComponent = renderResult.getByText('test1'); - expect(textComponent).not.toBeNull(); - expect(onRender).toHaveBeenCalledTimes(1); - - // Unmount the component and mount it again. It should now render immediately. - // Do not use `waitForPromisesToResolve` before making the assertions and make sure - // onRender was called. - renderResult.unmount(); - renderResult = render(); - - textComponent = renderResult.getByText('test1'); - expect(textComponent).not.toBeNull(); - expect(onRender).toHaveBeenCalledTimes(2); - }); - }); - - it('should cache missing keys', () => { - const TestComponentWithOnyx = withOnyx({ - text: { - key: ONYX_KEYS.TEST_KEY, - }, - })(ViewWithText); - const onRender = jest.fn(); - - // Render with a key that doesn't exist in cache or storage. - let renderResult = render(); - - // The component should not render initially since we have no data in cache. - // Use `waitForPromisesToResolve` before making the assertions and make sure - // onRender was not called. - expect(onRender).not.toHaveBeenCalled(); - return waitForPromisesToResolve().then(() => { - let textComponent = renderResult.getByText('null'); - expect(textComponent).not.toBeNull(); - expect(onRender).toHaveBeenCalledTimes(1); - - // Unmount the component and mount it again. It should now render immediately. - // Do not use `waitForPromisesToResolve` before making the assertions and make sure - // onRender was called. - renderResult.unmount(); - renderResult = render(); - textComponent = renderResult.getByText('null'); - expect(textComponent).not.toBeNull(); - expect(onRender).toHaveBeenCalledTimes(2); - }); - }); - - it('should cache empty collections', () => { - const TestComponentWithOnyx = withOnyx({ - text: { - key: ONYX_KEYS.COLLECTION.TEST_KEY, - }, - })(ViewWithCollections); - const onRender = jest.fn(); - let renderResult; - - // Set a random item in storage since Onyx will only think keys are loaded - // in cache if there are at least one key. - return StorageMock.setItem(ONYX_KEYS.SIMPLE_KEY, 'simple') - .then(() => { - // Render with a collection that doesn't exist in cache or storage. - renderResult = render(); - - // The component should not render initially since we have no data in cache. - // Use `waitForPromisesToResolve` before making the assertions and make sure - // onRender was not called. - expect(onRender).not.toHaveBeenCalled(); - - return waitForPromisesToResolve(); - }) - .then(() => { - let textComponent = renderResult.getByText('empty'); - expect(textComponent).not.toBeNull(); - expect(onRender).toHaveBeenCalledTimes(1); - - // Unmount the component and mount it again. It should now render immediately. - // Do not use `waitForPromisesToResolve` before making the assertions and make sure - // onRender was called. - renderResult.unmount(); - renderResult = render(); - textComponent = renderResult.getByText('empty'); - expect(textComponent).not.toBeNull(); - expect(onRender).toHaveBeenCalledTimes(2); - }); - }); });