resolve comments

Author: kubabutkiewicz

API-INTERNAL.md

@@ -71,7 +71,7 @@ is associated with a collection of keys.</p>
 <dd><p>Checks to see if a provided key is the exact configured key of our connected subscriber
 or if the provided key is a collection member key (in case our configured key is a &quot;collection key&quot;)</p>
 </dd>
-<dt><a href="#isSafeEvictionKey">isSafeEvictionKey()</a></dt>
+<dt><a href="#isEvictableKey">isEvictableKey()</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> ⇒</dt>
@@ -319,9 +319,9 @@ Checks to see if a provided key is the exact configured key of our connected sub
 or if the provided key is a collection member key (in case our configured key is a "collection key")
 
 **Kind**: global function  
-<a name="isSafeEvictionKey"></a>
+<a name="isEvictableKey"></a>
 
-## isSafeEvictionKey()
+## isEvictableKey()
 Checks to see if this key has been flagged as safe for removal.
 
 **Kind**: global function  

lib/OnyxCache.ts

@@ -79,7 +79,7 @@ class OnyxCache {
             'setAllKeys',
             'setEvictionAllowList',
             'getEvictionBlocklist',
-            'isSafeEvictionKey',
+            'isEvictableKey',
             'removeLastAccessedKey',
             'addLastAccessedKey',
             'addAllSafeEvictionKeysToRecentlyAccessedList',
@@ -254,12 +254,12 @@ class OnyxCache {
             const key = iterResult.value;
             // Don't consider the most recently accessed key for eviction
             // This ensures we don't immediately evict a key we just added
-            if (key !== undefined && key !== mostRecentKey && this.isSafeEvictionKey(key)) {
+            if (key !== undefined && key !== mostRecentKey && this.isEvictableKey(key)) {
                 safeKeysToRemove.push(key);
             }
             iterResult = iterator.next();
         }
-
+        console.log('safeKeysToRemove', safeKeysToRemove);
         safeKeysToRemove.forEach((key) => {
             delete this.storageMap[key];
             this.recentKeys.delete(key);
@@ -295,7 +295,7 @@ class OnyxCache {
      * Checks to see if this key has been flagged as safe for removal.
      * @param testKey - Key to check
      */
-    isSafeEvictionKey(testKey: OnyxKey): boolean {
+    isEvictableKey(testKey: OnyxKey): boolean {
         return this.evictionAllowList.some((key) => this.isKeyMatch(key, testKey));
     }
 
@@ -323,7 +323,7 @@ class OnyxCache {
      */
     addLastAccessedKey(key: OnyxKey, isCollectionKey: boolean): void {
         // Only specific keys belong in this list since we cannot remove an entire collection.
-        if (isCollectionKey || !this.isSafeEvictionKey(key)) {
+        if (isCollectionKey || !this.isEvictableKey(key)) {
             return;
         }
 

lib/OnyxUtils.ts

@@ -112,13 +112,6 @@ function getDeferredInitTask(): DeferredTask {
     return deferredInitTask;
 }
 
-/**
- * Getter - returns the eviction block list.
- */
-function getEvictionBlocklist(): Record<OnyxKey, string[] | undefined> {
-    return cache.getEvictionBlocklist();
-}
-
 /**
  * Getter - returns the skippable collection member IDs.
  */
@@ -1011,7 +1004,7 @@ function sendDataToConnection<TKey extends OnyxKey>(mapping: Mapping<TKey>, valu
  * run out of storage the least recently accessed key can be removed.
  */
 function addKeyToRecentlyAccessedIfNeeded<TKey extends OnyxKey>(mapping: Mapping<TKey>): void {
-    if (!cache.isSafeEvictionKey(mapping.key)) {
+    if (!cache.isEvictableKey(mapping.key)) {
         return;
     }
 
@@ -1424,7 +1417,6 @@ const OnyxUtils = {
     doAllCollectionItemsBelongToSameParent,
     subscribeToKey,
     unsubscribeFromKey,
-    getEvictionBlocklist,
     getSkippableCollectionMemberIDs,
     setSkippableCollectionMemberIDs,
 };

lib/useOnyx.ts

@@ -209,7 +209,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
             return;
         }
 
-        if (!OnyxCache.isSafeEvictionKey(key)) {
+        if (!OnyxCache.isEvictableKey(key)) {
             throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({evictableKeys: []}).`);
         }
 

lib/withOnyx/index.tsx

@@ -289,7 +289,7 @@ export default function <TComponentProps, TOnyxProps>(
                     const canEvict = !!Str.result(mapping.canEvict as GenericFunction, this.props);
                     const key = Str.result(mapping.key as GenericFunction, this.props);
 
-                    if (!cache.isSafeEvictionKey(key)) {
+                    if (!cache.isEvictableKey(key)) {
                         throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({evictableKeys: []}).`);
                     }
 

tests/unit/cacheEvictionTest.ts

@@ -50,12 +50,10 @@ test('Cache eviction', () => {
             );
             StorageMock.setItem = setItemMock;
 
-            return Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}${RECORD_TO_ADD}`, {test: 'add'})
-                .then(() => waitForPromisesToResolve())
-                .then(() => {
-                    // Then our collection should no longer contain the evictable key
-                    expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}${RECORD_TO_EVICT}`]).toBe(undefined);
-                    expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}${RECORD_TO_ADD}`]).toStrictEqual({test: 'add'});
-                });
+            return Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}${RECORD_TO_ADD}`, {test: 'add'}).then(() => {
+                // Then our collection should no longer contain the evictable key
+                expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}${RECORD_TO_EVICT}`]).toBe(undefined);
+                expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}${RECORD_TO_ADD}`]).toStrictEqual({test: 'add'});
+            });
         });
 });

tests/unit/onyxCacheTest.tsx

@@ -535,9 +535,7 @@ describe('Onyx', () => {
             let connections: Array<{key: string; connection: Connection}> = [];
 
             // Given Onyx is configured with max 5 keys in cache
-            return initOnyx({
-                maxCachedKeysCount: 5,
-            })
+            return initOnyx({maxCachedKeysCount: 5})
                 .then(() => {
                     // Given 10 connections for different keys
                     connections = range.map((number) => {
@@ -709,11 +707,11 @@ describe('Onyx', () => {
             })
                 .then(() => {
                     // Verify keys are correctly identified as evictable or not
-                    expect(cache.isSafeEvictionKey?.(evictableKey1)).toBe(true);
-                    expect(cache.isSafeEvictionKey?.(evictableKey2)).toBe(true);
-                    expect(cache.isSafeEvictionKey?.(evictableKey3)).toBe(true);
-                    expect(cache.isSafeEvictionKey?.(triggerKey)).toBe(true);
-                    expect(cache.isSafeEvictionKey?.(criticalKey1)).toBe(false);
+                    expect(cache.isEvictableKey?.(evictableKey1)).toBe(true);
+                    expect(cache.isEvictableKey?.(evictableKey2)).toBe(true);
+                    expect(cache.isEvictableKey?.(evictableKey3)).toBe(true);
+                    expect(cache.isEvictableKey?.(triggerKey)).toBe(true);
+                    expect(cache.isEvictableKey?.(criticalKey1)).toBe(false);
 
                     // Connect to non-evictable keys first
                     Onyx.connect({key: criticalKey1, callback: jest.fn()});