Skip to content

Commit

Permalink
Revert "Merge pull request #401 from janicduplessis/@janic/cache-fixes"
Browse files Browse the repository at this point in the history 
This reverts commit a8c0115, reversing
changes made to 9672073.
  • Loading branch information
@tgolen
tgolen committed Oct 27, 2023
1 parent 40c5b68 commit 94372ef
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 129 deletions.
18 changes: 5 additions & 13 deletions lib/Onyx.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -252,7 +246,9 @@ function tryGetCachedValue(key, mapping = {}) {
}
return finalObject;
}, {});

if (_.isEmpty(values)) {
return;
}
val = values;
}

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions tests/components/ViewWithCollections.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ 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: 2 additions & 3 deletions tests/components/ViewWithText.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,20 @@ 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) {
props.onRender();

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

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

// 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(<TestComponentWithOnyx onRender={onRender} />);
textComponent = renderResult.getByText('empty');
expect(textComponent).not.toBeNull();
expect(onRender).toHaveBeenCalledTimes(2);
});
});
});

0 comments on commit 94372ef

Please sign in to comment.