Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion lib/DevTools/RealDevTools.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {IDevTools, DevtoolsOptions, DevtoolsConnection, ReduxDevtools} from './types';
import OnyxUtils from '../OnyxUtils';

const ERROR_LABEL = 'Onyx DevTools - Error: ';

Expand Down Expand Up @@ -76,7 +77,7 @@ class RealDevTools implements IDevTools {
clearState(keysToPreserve: string[] = []): void {
const newState = Object.entries(this.state).reduce((obj: Record<string, unknown>, [key, value]) => {
// eslint-disable-next-line no-param-reassign
obj[key] = keysToPreserve.includes(key) ? value : this.defaultState[key];
obj[key] = keysToPreserve.some((preserveKey) => OnyxUtils.isKeyMatch(preserveKey, key)) ? value : this.defaultState[key];
return obj;
}, {});

Expand Down
4 changes: 2 additions & 2 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
// to null would cause unknown behavior)
// 2.1 However, if a default key was explicitly set to null, we need to reset it to the default value
for (const key of allKeys) {
const isKeyToPreserve = keysToPreserve.includes(key);
const isKeyToPreserve = keysToPreserve.some((preserveKey) => OnyxUtils.isKeyMatch(preserveKey, key));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restrict preserved collection keys to their exact collection

If an app defines overlapping collection prefixes (which this repo explicitly supports in tests/unit/onyxUtilsTest.ts:73-78, e.g. test_ and test_level_), OnyxUtils.isKeyMatch() will treat test_level_1 as a match for test_ because it only checks startsWith (lib/OnyxUtils.ts:557-559). After this change, Onyx.clear([someCollectionKey]) will therefore keep members of any longer collection whose prefix starts the same way, so unrelated data can survive a clear/sign-out whenever collection keys overlap.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is true, though I'm not sure it's actually a problem

Copy link
Copy Markdown
Contributor

@fabioh8010 fabioh8010 Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a safe change because you are passing the collection key to isKeyMatch and it also check if the passed key is a collection one

const isDefaultKey = key in defaultKeyStates;

// If the key is being removed or reset to default:
Expand Down Expand Up @@ -382,7 +382,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
// Exclude RAM-only keys to prevent them from being saved to storage
const defaultKeyValuePairs = Object.entries(
Object.keys(defaultKeyStates)
.filter((key) => !keysToPreserve.includes(key) && !OnyxUtils.isRamOnlyKey(key))
.filter((key) => !keysToPreserve.some((preserveKey) => OnyxUtils.isKeyMatch(preserveKey, key)) && !OnyxUtils.isRamOnlyKey(key))
.reduce((obj: KeyValueMapping, key) => {
// eslint-disable-next-line no-param-reassign
obj[key] = defaultKeyStates[key];
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/DevToolsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,12 @@ describe('DevTools', () => {
const devToolsInstance = getDevToolsInstance() as RealDevToolsType;
expect(devToolsInstance['state']).toEqual({...initialKeyStates, [ONYX_KEYS.NUM_KEY]: 2});
});

it('Preserves collection member keys when a collection key is passed to keysToPreserve', async () => {
await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.NUM_KEY, exampleCollection);
await Onyx.clear([ONYX_KEYS.COLLECTION.NUM_KEY]);
const devToolsInstance = getDevToolsInstance() as RealDevToolsType;
expect(devToolsInstance['state']).toEqual({...initialKeyStates, ...exampleCollection});
});
});
});
81 changes: 81 additions & 0 deletions tests/unit/onyxClearWebStorageTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,87 @@ describe('Set data while storage is clearing', () => {
});
});

it('should preserve all collection members when a collection key is passed to keysToPreserve', () => {
expect.assertions(6);

const collectionItemKey1 = `${ONYX_KEYS.COLLECTION.TEST}1`;
const collectionItemKey2 = `${ONYX_KEYS.COLLECTION.TEST}2`;

// Given that Onyx has a collection with two items
return (
Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST, {
[collectionItemKey1]: {id: 1, name: 'first'},
[collectionItemKey2]: {id: 2, name: 'second'},
} as GenericCollection)
// When clear is called with the collection prefix as a key to preserve
.then(() => Onyx.clear([ONYX_KEYS.COLLECTION.TEST]))
.then(() => waitForPromisesToResolve())
.then(() => {
// Then both collection members are preserved in the cache and storage
expect(cache.get(collectionItemKey1)).toEqual({id: 1, name: 'first'});
expect(cache.get(collectionItemKey2)).toEqual({id: 2, name: 'second'});

return Promise.all([StorageMock.getItem(collectionItemKey1), StorageMock.getItem(collectionItemKey2)]);
})
.then(([storedValue1, storedValue2]) => {
expect(storedValue1).toEqual({id: 1, name: 'first'});
expect(storedValue2).toEqual({id: 2, name: 'second'});

// And non-collection keys are still cleared (default key reset to default)
expect(cache.get(ONYX_KEYS.DEFAULT_KEY)).toBe(DEFAULT_VALUE);
return expect(StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY)).resolves.toBe(DEFAULT_VALUE);
})
);
});

it('should preserve collection members and still clear regular keys not in keysToPreserve', () => {
expect.assertions(4);

const collectionItemKey1 = `${ONYX_KEYS.COLLECTION.TEST}1`;

// Given that Onyx has both a collection item and a regular key set
return (
Promise.all([Onyx.set(ONYX_KEYS.REGULAR_KEY, SET_VALUE), Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST, {[collectionItemKey1]: 'value'} as GenericCollection)])
// When clear is called preserving only the collection
.then(() => Onyx.clear([ONYX_KEYS.COLLECTION.TEST]))
.then(() => waitForPromisesToResolve())
.then(() => {
// Then the collection member is preserved
expect(cache.get(collectionItemKey1)).toBe('value');
return expect(StorageMock.getItem(collectionItemKey1)).resolves.toBe('value');
})
.then(() => {
// And the regular key is cleared
expect(cache.get(ONYX_KEYS.REGULAR_KEY)).toBeUndefined();
return expect(StorageMock.getItem(ONYX_KEYS.REGULAR_KEY)).resolves.toBeNull();
})
);
});

it('should preserve both collection keys and individual keys when both are passed to keysToPreserve', () => {
expect.assertions(4);

const collectionItemKey1 = `${ONYX_KEYS.COLLECTION.TEST}1`;

// Given that Onyx has a collection item and a regular key set
return (
Promise.all([Onyx.set(ONYX_KEYS.REGULAR_KEY, SET_VALUE), Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST, {[collectionItemKey1]: 'value'} as GenericCollection)])
// When clear is called preserving both the collection and the regular key
.then(() => Onyx.clear([ONYX_KEYS.COLLECTION.TEST, ONYX_KEYS.REGULAR_KEY]))
.then(() => waitForPromisesToResolve())
.then(() => {
// Then both the collection member and the regular key are preserved
expect(cache.get(collectionItemKey1)).toBe('value');
expect(cache.get(ONYX_KEYS.REGULAR_KEY)).toBe(SET_VALUE);
return Promise.all([StorageMock.getItem(collectionItemKey1), StorageMock.getItem(ONYX_KEYS.REGULAR_KEY)]);
})
.then(([storedCollectionValue, storedRegularValue]) => {
expect(storedCollectionValue).toBe('value');
expect(storedRegularValue).toBe(SET_VALUE);
})
);
});

it('should only trigger the connection callback once when using wait for collection callback', () => {
expect.assertions(4);

Expand Down
Loading