diff --git a/lib/Onyx.js b/lib/Onyx.js index a940afc92..7bb3f4e7e 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -235,8 +235,14 @@ function tryGetCachedValue(key, mapping = {}) { let val = cache.getValue(key); if (isCollectionKey(key)) { - const allKeys = cache.getAllKeys(); - const matchingKeys = _.filter(allKeys, k => k.startsWith(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 values = _.reduce(matchingKeys, (finalObject, matchedKey) => { const cachedValue = cache.getValue(matchedKey); if (cachedValue) { @@ -246,9 +252,7 @@ function tryGetCachedValue(key, mapping = {}) { } return finalObject; }, {}); - if (_.isEmpty(values)) { - return; - } + val = values; } @@ -826,6 +830,10 @@ 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 ecf380efc..aa05d0b93 100644 --- a/tests/components/ViewWithCollections.js +++ b/tests/components/ViewWithCollections.js @@ -29,6 +29,10 @@ 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 e172f2953..0a8720777 100644 --- a/tests/components/ViewWithText.js +++ b/tests/components/ViewWithText.js @@ -4,12 +4,13 @@ import PropTypes from 'prop-types'; import {View, Text} from 'react-native'; const propTypes = { - text: PropTypes.string.isRequired, + text: PropTypes.string, onRender: PropTypes.func, }; const defaultProps = { onRender: () => {}, + text: null, }; function ViewWithText(props) { @@ -17,7 +18,7 @@ function ViewWithText(props) { return ( - {props.text} + {props.text || 'null'} ); } diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 82fc544b5..56207dbde 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -6,6 +6,7 @@ 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', @@ -522,4 +523,112 @@ 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); + }); + }); });