Merge pull request #750 from callstack-internal/JKobrynski/feat/remove-allow-dynamic-key-poc

Remove allowDynamicKey

Author: luacmartins

lib/OnyxSnapshotCache.ts

@@ -60,7 +60,7 @@ class OnyxSnapshotCache {
      * - `selector`: Different selectors produce different results, so each selector needs its own cache entry
      * - `initWithStoredValues`: This flag changes the initial loading behavior and affects the returned fetch status
      *
-     * Other options like `canEvict`, `reuseConnection`, and `allowDynamicKey` don't affect the data transformation
+     * Other options like `canEvict` and `reuseConnection` don't affect the data transformation
      * or timing behavior of getSnapshot, so they're excluded from the cache key for better cache hit rates.
      */
     registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues'>): string {

lib/useOnyx.ts

@@ -32,11 +32,6 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
      */
     reuseConnection?: boolean;
 
-    /**
-     * If set to `true`, the key can be changed dynamically during the component lifecycle.
-     */
-    allowDynamicKey?: boolean;
-
     /**
      * This will be used to subscribe to a subset of an Onyx key's data.
      * Using this setting on `useOnyx` can have very positive performance benefits because the component will only re-render
@@ -147,30 +142,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
 
     useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]);
 
-    useEffect(() => {
-        // These conditions will ensure we can only handle dynamic collection member keys from the same collection.
-        if (options?.allowDynamicKey || previousKey === key) {
-            return;
-        }
-
-        try {
-            const previousCollectionKey = OnyxUtils.splitCollectionMemberKey(previousKey)[0];
-            const collectionKey = OnyxUtils.splitCollectionMemberKey(key)[0];
-
-            if (OnyxUtils.isCollectionMemberKey(previousCollectionKey, previousKey) && OnyxUtils.isCollectionMemberKey(collectionKey, key) && previousCollectionKey === collectionKey) {
-                return;
-            }
-        } catch (e) {
-            throw new Error(
-                `'${previousKey}' key can't be changed to '${key}'. useOnyx() only supports dynamic keys if they are both collection member keys from the same collection e.g. from 'collection_id1' to 'collection_id2'.`,
-            );
-        }
-
-        throw new Error(
-            `'${previousKey}' key can't be changed to '${key}'. useOnyx() only supports dynamic keys if they are both collection member keys from the same collection e.g. from 'collection_id1' to 'collection_id2'.`,
-        );
-    }, [previousKey, key, options?.allowDynamicKey]);
-
     // Track previous dependencies to prevent infinite loops
     const previousDependenciesRef = useRef<DependencyList>([]);
 

tests/unit/useOnyxTest.ts

@@ -35,111 +35,6 @@ beforeEach(async () => {
 });
 
 describe('useOnyx', () => {
-    describe('dynamic key', () => {
-        const error = (key1: string, key2: string) =>
-            `'${key1}' key can't be changed to '${key2}'. useOnyx() only supports dynamic keys if they are both collection member keys from the same collection e.g. from 'collection_id1' to 'collection_id2'.`;
-
-        beforeEach(() => {
-            jest.spyOn(console, 'error').mockImplementation(jest.fn);
-        });
-
-        afterEach(() => {
-            (console.error as unknown as jest.SpyInstance<void, Parameters<typeof console.error>>).mockRestore();
-        });
-
-        it('should throw an error when changing from a non-collection key to another one', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key), {initialProps: ONYXKEYS.TEST_KEY});
-
-            try {
-                await act(async () => {
-                    rerender(ONYXKEYS.TEST_KEY_2);
-                });
-
-                fail('Expected to throw an error.');
-            } catch (e) {
-                expect((e as Error).message).toBe(error(ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_2));
-            }
-        });
-
-        it('should throw an error when changing from a collection key to another one', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key), {initialProps: ONYXKEYS.COLLECTION.TEST_KEY});
-
-            try {
-                await act(async () => {
-                    rerender(ONYXKEYS.COLLECTION.TEST_KEY_2);
-                });
-
-                fail('Expected to throw an error.');
-            } catch (e) {
-                expect((e as Error).message).toBe(error(ONYXKEYS.COLLECTION.TEST_KEY, ONYXKEYS.COLLECTION.TEST_KEY_2));
-            }
-        });
-
-        it('should throw an error when changing from a collection key to a collectiom member key', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key), {initialProps: ONYXKEYS.COLLECTION.TEST_KEY});
-
-            try {
-                await act(async () => {
-                    rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}1`);
-                });
-
-                fail('Expected to throw an error.');
-            } catch (e) {
-                expect((e as Error).message).toBe(error(ONYXKEYS.COLLECTION.TEST_KEY, `${ONYXKEYS.COLLECTION.TEST_KEY}1`));
-            }
-        });
-
-        it('should not throw any errors when changing from a collection member key to another one', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
-
-            try {
-                await act(async () => {
-                    rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
-                });
-            } catch (e) {
-                fail("Expected to don't throw any errors.");
-            }
-        });
-
-        it('should not throw an error when changing from a non-collection key to another one if allowDynamicKey is true', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key, {allowDynamicKey: true}), {initialProps: ONYXKEYS.TEST_KEY});
-
-            try {
-                await act(async () => {
-                    rerender(ONYXKEYS.TEST_KEY_2);
-                });
-            } catch (e) {
-                fail("Expected to don't throw any errors.");
-            }
-        });
-
-        it('should throw an error when changing from a non-collection key to another one if allowDynamicKey is false', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key, {allowDynamicKey: false}), {initialProps: ONYXKEYS.TEST_KEY});
-
-            try {
-                await act(async () => {
-                    rerender(ONYXKEYS.TEST_KEY_2);
-                });
-
-                fail('Expected to throw an error.');
-            } catch (e) {
-                expect((e as Error).message).toBe(error(ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_2));
-            }
-        });
-
-        it('should not throw an error when changing from a collection member key to another one if allowDynamicKey is true', async () => {
-            const {rerender} = renderHook((key: string) => useOnyx(key, {allowDynamicKey: true}), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}` as string});
-
-            try {
-                await act(async () => {
-                    rerender(`${ONYXKEYS.COLLECTION.TEST_KEY_2}`);
-                });
-            } catch (e) {
-                fail("Expected to don't throw any errors.");
-            }
-        });
-    });
-
     describe('misc', () => {
         it('should initially return loading state while loading non-existent key, and then return `undefined` and loaded state', async () => {
             const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));