Skip to content

Commit

Permalink
Fix cache for missing keys and empty collections
Browse files Browse the repository at this point in the history
  • Loading branch information
janicduplessis committed Oct 13, 2023
1 parent 9672073 commit 7b98cca
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 5 deletions.
14 changes: 11 additions & 3 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,12 @@ function tryGetCachedValue(key, mapping = {}) {

if (isCollectionKey(key)) {
const allKeys = cache.getAllKeys();

// It is possible we haven't loaded all keys yet so we do not know if the
// collection actually exists.
if (allKeys.length === 0) {
return;
}
const matchingKeys = _.filter(allKeys, k => k.startsWith(key));
const values = _.reduce(matchingKeys, (finalObject, matchedKey) => {
const cachedValue = cache.getValue(matchedKey);
Expand All @@ -246,9 +252,7 @@ function tryGetCachedValue(key, mapping = {}) {
}
return finalObject;
}, {});
if (_.isEmpty(values)) {
return;
}

val = values;
}

Expand Down Expand Up @@ -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) {
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);
Expand Down
4 changes: 4 additions & 0 deletions tests/components/ViewWithCollections.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ const ViewWithCollections = React.forwardRef((props, ref) => {

props.onRender(props);

if (_.size(props.collections) === 0) {
return <Text>empty</Text>;
}

return (
<View>
{_.map(props.collections, (collection, i) => (
Expand Down
5 changes: 3 additions & 2 deletions tests/components/ViewWithText.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,21 @@ 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) {
props.onRender();

return (
<View>
<Text testID="text-element">{props.text}</Text>
<Text testID="text-element">{props.text || 'null'}</Text>
</View>
);
}
Expand Down
90 changes: 90 additions & 0 deletions tests/unit/withOnyxTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -522,4 +523,93 @@ 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(<TestComponentWithOnyx onRender={onRender} />);

// The component should not render initially since we have no data in cache.
expect(onRender).not.toHaveBeenCalled();
return 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.
renderResult.unmount();
renderResult = render(<TestComponentWithOnyx onRender={onRender} />);

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(<TestComponentWithOnyx onRender={onRender} />);

// The component should not render initially since we have no data in cache.
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.
renderResult.unmount();
renderResult = render(<TestComponentWithOnyx onRender={onRender} />);
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();

// Render with a collection that doesn't exist in cache or storage.
let renderResult = render(<TestComponentWithOnyx onRender={onRender} />);

// The component should not render initially since we have no data in cache.
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.
renderResult.unmount();
renderResult = render(<TestComponentWithOnyx onRender={onRender} />);
textComponent = renderResult.getByText('empty');
expect(textComponent).not.toBeNull();
expect(onRender).toHaveBeenCalledTimes(2);
});
});
});

0 comments on commit 7b98cca

Please sign in to comment.