Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add setCollection method #594

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 24 additions & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ value will be saved to storage after the default value.</p>
<dt><a href="#update">update(data)</a> ⇒</dt>
<dd><p>Insert API responses and lifecycle data into Onyx</p>
</dd>
<dt><a href="#setCollection">setCollection(collectionKey, collection)</a></dt>
<dd><p>Sets a collection by replacing all existing collection members with new values.
Any existing collection members not included in the new data will be removed.</p>
</dd>
</dl>

<a name="init"></a>
Expand Down Expand Up @@ -209,3 +213,23 @@ Insert API responses and lifecycle data into Onyx
| --- | --- |
| data | An array of objects with update expressions |

<a name="setCollection"></a>

## setCollection(collectionKey, collection)
Sets a collection by replacing all existing collection members with new values.
Any existing collection members not included in the new data will be removed.

**Kind**: global function

| Param | Description |
| --- | --- |
| collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
| collection | Object collection keyed by individual collection member keys and values |

**Example**
```js
Onyx.setCollection(ONYXKEYS.COLLECTION.REPORT, {
[`${ONYXKEYS.COLLECTION.REPORT}1`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}2`]: report2,
});
```
51 changes: 51 additions & 0 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,56 @@ function update(data: OnyxUpdate[]): Promise<void> {
.then(() => undefined);
}

/**
* Sets a collection by replacing all existing collection members with new values.
* Any existing collection members not included in the new data will be removed.
*
* @example
* Onyx.setCollection(ONYXKEYS.COLLECTION.REPORT, {
* [`${ONYXKEYS.COLLECTION.REPORT}1`]: report1,
* [`${ONYXKEYS.COLLECTION.REPORT}2`]: report2,
* });
*
* @param collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT`
* @param collection Object collection keyed by individual collection member keys and values
*/
function setCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TKey, collection: OnyxMergeCollectionInput<TKey, TMap>): Promise<void> {
if (!OnyxUtils.isValidNonEmptyCollectionForMerge(collection)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's totally valid to set a collection to empty if you want to clear it, not sure why we would prevent that

Logger.logInfo('setCollection() called with invalid or empty value. Skipping this update.');
return Promise.resolve();
}

const newCollectionKeys = Object.keys(collection);
if (!OnyxUtils.doAllCollectionItemsBelongToSameParent(collectionKey, newCollectionKeys)) {
return Promise.resolve();
zirgulis marked this conversation as resolved.
Show resolved Hide resolved
}

return OnyxUtils.getAllKeys().then((persistedKeys) => {
// Find existing collection members
const existingCollectionKeys = Array.from(persistedKeys).filter((key) => key.startsWith(collectionKey));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this creating a potentially huge array from the set only to select a few entries from it and discard the set? Why not iterate the set and get the entries we want, reducing the memory footprint?


// Create removal object with null values for keys to be removed
const removalCollection = existingCollectionKeys
.filter((key) => !newCollectionKeys.includes(key))
.reduce(
(obj, key) => ({
...obj,
[key]: null,
}),
{},
);

// Combine removals with new collection data
const finalCollection = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here, isn't this creating a new variable with the combination of both removalCollection and collection and thus using more memory and CPU and instead would be better to avoid that by adding the removalCollection keys to the collection directly?

...removalCollection,
...collection,
};

// Use multiSet to handle all updates atomically
return multiSet(finalCollection);
});
}

const Onyx = {
METHOD: OnyxUtils.METHOD,
connect,
Expand All @@ -733,6 +783,7 @@ const Onyx = {
multiSet,
merge,
mergeCollection,
setCollection,
update,
clear,
init,
Expand Down
1 change: 1 addition & 0 deletions lib/OnyxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const METHOD = {
SET: 'set',
MERGE: 'merge',
MERGE_COLLECTION: 'mergecollection',
SET_COLLECTION: 'setcollection',
MULTI_SET: 'multiset',
CLEAR: 'clear',
} as const;
Expand Down
81 changes: 81 additions & 0 deletions tests/unit/onyxTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1725,5 +1725,86 @@ describe('Onyx', () => {
});
});
});

describe('setCollection', () => {
it('should replace all existing collection members with new values and remove old ones', async () => {
let result: OnyxCollection<unknown>;
const routeA = `${ONYX_KEYS.COLLECTION.ROUTES}A`;
const routeB = `${ONYX_KEYS.COLLECTION.ROUTES}B`;
const routeC = `${ONYX_KEYS.COLLECTION.ROUTES}C`;

connection = Onyx.connect({
key: ONYX_KEYS.COLLECTION.ROUTES,
initWithStoredValues: false,
callback: (value) => (result = value),
waitForCollectionCallback: true,
});

// Set initial collection state
await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, {
[routeA]: {name: 'Route A'},
[routeB]: {name: 'Route B'},
[routeC]: {name: 'Route C'},
} as GenericCollection);

// Replace with new collection data
await Onyx.setCollection(ONYX_KEYS.COLLECTION.ROUTES, {
[routeA]: {name: 'New Route A'},
[routeB]: {name: 'New Route B'},
} as GenericCollection);

expect(result).toEqual({
[routeA]: {name: 'New Route A'},
[routeB]: {name: 'New Route B'},
});
});

it('should not update if collection is empty', async () => {
let result: OnyxCollection<unknown>;
const routeA = `${ONYX_KEYS.COLLECTION.ROUTES}A`;

connection = Onyx.connect({
key: ONYX_KEYS.COLLECTION.ROUTES,
initWithStoredValues: false,
callback: (value) => (result = value),
waitForCollectionCallback: true,
});

await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, {
[routeA]: {name: 'Route A'},
} as GenericCollection);

await Onyx.setCollection(ONYX_KEYS.COLLECTION.ROUTES, {} as GenericCollection);

expect(result).toEqual({
[routeA]: {name: 'Route A'},
});
});

it('should reject collection items with invalid keys', async () => {
let result: OnyxCollection<unknown>;
const routeA = `${ONYX_KEYS.COLLECTION.ROUTES}A`;
const invalidRoute = 'invalid_route';

connection = Onyx.connect({
key: ONYX_KEYS.COLLECTION.ROUTES,
initWithStoredValues: false,
callback: (value) => (result = value),
waitForCollectionCallback: true,
});

await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, {
[routeA]: {name: 'Route A'},
} as GenericCollection);

await Onyx.setCollection(ONYX_KEYS.COLLECTION.ROUTES, {
[invalidRoute]: {name: 'Invalid Route'},
} as GenericCollection);

expect(result).toEqual({
[routeA]: {name: 'Route A'},
});
});
});
});
});
Loading