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

Improve Onyx functions docs #582

Merged
merged 2 commits into from
Sep 17, 2024
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
19 changes: 10 additions & 9 deletions API-INTERNAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ or if the provided key is a collection member key (in case our configured key is
<dt><a href="#isSafeEvictionKey">isSafeEvictionKey()</a></dt>
<dd><p>Checks to see if this key has been flagged as safe for removal.</p>
</dd>
<dt><a href="#getCollectionKey">getCollectionKey(key)</a> ⇒ <code>string</code></dt>
<dd><p>It extracts the non-numeric collection identifier of a given key.</p>
<dt><a href="#getCollectionKey">getCollectionKey(key)</a> ⇒</dt>
<dd><p>Extracts the collection identifier of a given collection member key.</p>
<p>For example:</p>
<ul>
<li><code>getCollectionKey(&quot;report_123&quot;)</code> would return &quot;report_&quot;</li>
Expand Down Expand Up @@ -272,7 +272,8 @@ is associated with a collection of keys.
Splits a collection member key into the collection key part and the ID part.

**Kind**: global function
**Returns**: A tuple where the first element is the collection part and the second element is the ID part.
**Returns**: A tuple where the first element is the collection part and the second element is the ID part,
or throws an Error if the key is not a collection one.

| Param | Description |
| --- | --- |
Expand All @@ -293,8 +294,8 @@ Checks to see if this key has been flagged as safe for removal.
**Kind**: global function
<a name="getCollectionKey"></a>

## getCollectionKey(key) ⇒ <code>string</code>
It extracts the non-numeric collection identifier of a given key.
## getCollectionKey(key) ⇒
Extracts the collection identifier of a given collection member key.

For example:
- `getCollectionKey("report_123")` would return "report_"
Expand All @@ -303,11 +304,11 @@ For example:
- `getCollectionKey("sharedNVP_user_-1_something")` would return "sharedNVP_user_"

**Kind**: global function
**Returns**: <code>string</code> - The plain collection key.
**Returns**: The plain collection key or throws an Error if the key is not a collection one.

| Param | Type | Description |
| --- | --- | --- |
| key | <code>OnyxKey</code> | The key to process. |
| Param | Description |
| --- | --- |
| key | The collection key to process. |

<a name="tryGetCachedValue"></a>

Expand Down
13 changes: 7 additions & 6 deletions lib/OnyxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,8 @@ function isCollectionMemberKey<TCollectionKey extends CollectionKeyBase>(collect
/**
* Splits a collection member key into the collection key part and the ID part.
* @param key - The collection member key to split.
* @returns A tuple where the first element is the collection part and the second element is the ID part.
* @returns A tuple where the first element is the collection part and the second element is the ID part,
* or throws an Error if the key is not a collection one.
*/
function splitCollectionMemberKey<TKey extends CollectionKey, CollectionKeyType = TKey extends `${infer Prefix}_${string}` ? `${Prefix}_` : never>(key: TKey): [CollectionKeyType, string] {
const collectionKey = getCollectionKey(key);
Expand All @@ -439,18 +440,18 @@ function isSafeEvictionKey(testKey: OnyxKey): boolean {
}

/**
* It extracts the non-numeric collection identifier of a given key.
* Extracts the collection identifier of a given collection member key.
*
* For example:
* - `getCollectionKey("report_123")` would return "report_"
* - `getCollectionKey("report_")` would return "report_"
* - `getCollectionKey("report_-1_something")` would return "report_"
* - `getCollectionKey("sharedNVP_user_-1_something")` would return "sharedNVP_user_"
*
* @param {OnyxKey} key - The key to process.
* @return {string} The plain collection key.
* @param key - The collection key to process.
* @returns The plain collection key or throws an Error if the key is not a collection one.
*/
function getCollectionKey(key: OnyxKey): string {
function getCollectionKey(key: CollectionKey): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct to use CollectionKey if it might not be a collection key and we will throw in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We try to avoid the situation by putting a stricter type in the function, but someone can still bypass this and pass an incorrect key, which will lead to an Error being thrown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have similar strict typing for splitCollectionMemberKey

// Start by finding the position of the last underscore in the string
let lastUnderscoreIndex = key.lastIndexOf('_');

Expand Down Expand Up @@ -1280,7 +1281,7 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
// Performance improvement
// If the mapping is connected to an onyx key that is not a collection
// we can skip the call to getAllKeys() and return an array with a single item
if (Boolean(mapping.key) && typeof mapping.key === 'string' && !mapping.key.endsWith('_') && cache.getAllKeys().has(mapping.key)) {
if (Boolean(mapping.key) && typeof mapping.key === 'string' && !isCollectionKey(mapping.key) && cache.getAllKeys().has(mapping.key)) {
return new Set([mapping.key]);
}
return getAllKeys();
Expand Down
Loading