Fixed executor subscription

Author: smikhalevski

README.md

@@ -112,13 +112,13 @@ npm install --save-prod react-executor
 
 # Introduction
 
-An executor executes a task, stores the execution result, and provides access to it. Tasks are callbacks that return a
-value or throw an error.
+An executor runs a task, stores the execution result, and provides access to that result. Tasks are callbacks that
+return a value or throw an error.
 
 An [`Executor`](https://smikhalevski.github.io/react-executor/interfaces/react-executor.Executor.html) is created and
 managed by
-an [`ExecutorManager`](https://smikhalevski.github.io/react-executor/classes/react-executor.ExecutorManager.html)
-which controls the executor lifecycle:
+an [`ExecutorManager`](https://smikhalevski.github.io/react-executor/classes/react-executor.ExecutorManager.html),
+which controls the executor's lifecycle:
 
 ```ts
 import { ExecutorManager } from 'react-executor';
@@ -129,12 +129,12 @@ const rookyExecutor = manager.getOrCreate('rooky');
 // ⮕ Executor<any>
 ```
 
-Each executor has a unique key in the scope of the manager. Here we created the new executor with the key `'rooky'`.
-Managers create a new executor when you call
+Each executor has a unique key within the manager's scope. In this example, we created a new executor with the key
+`'rooky'`. The manager creates a new executor when you call
 [`getOrCreate`](https://smikhalevski.github.io/react-executor/classes/react-executor.ExecutorManager.html#getorcreate)
-with a new key. Each consequent call with that key returns the same executor.
+with a previously unused key. Subsequent calls with the same key return the same executor instance.
 
-If you want to retrieve an existing executor by its key and don't want to create a new executor if it doesn't exist, use
+If you want to retrieve an existing executor by its key and avoid creating a new one if it doesn't exist, use
 [`get`](https://smikhalevski.github.io/react-executor/classes/react-executor.ExecutorManager.html#get):
 
 ```ts
@@ -145,7 +145,7 @@ manager.get('rooky');
 // ⮕ Executor<any>
 ```
 
-The executor we created is unsettled, which means it neither stores a value, nor a task failure reason:
+The executor we created is _unsettled_, meaning it stores neither a value nor a failure reason:
 
 ```ts
 rookyExecutor.isSettled;
@@ -168,19 +168,19 @@ bobbyExecutor.value;
 // ⮕ 42
 ```
 
-An initial value can be a task which is executed, a promise which the executor awaits, or any other value that instantly
-fulfills the executor. Read more in the [Execute a task](#execute-a-task) and in
+An initial value may be a task (which will be executed), a promise (which the executor will await), or any other value
+that immediately fulfills the executor. Read more in the [Execute a task](#execute-a-task) and in
 the [Settle an executor](#settle-an-executor) sections.
 
-When an executor is created, you can provide an array of plugins:
+When creating an executor, you can also provide an array of plugins:
 
 ```ts
 import retryRejected from 'react-executor/plugin/retryRejected';
 
 const rookyExecutor = manager.getOrCreate('rooky', 42, [retryRejected()]);
 ```
 
-Plugins can subscribe to [executor events](#events-and-lifecycle) or alter the executor instance. Read more about
+Plugins can subscribe to [executor events](#events-and-lifecycle) or modify the executor instance. Read more about
 plugins in the [Plugins](#plugins) section.
 
 ## Executor keys
@@ -2215,24 +2215,20 @@ To detect a global pending state we can rely on events published by
 an [`ExecutorManager`](https://smikhalevski.github.io/react-executor/classes/react-executor.ExecutorManager.html):
 
 ```ts
-function useGlobalPending(predicate = (executor: Executor) => true): boolean {
+function useIsPending(predicate = (executor: Executor) => true): boolean {
   const manager = useExecutorManager();
   const [isPending, setPending] = useState(false);
 
   useEffect(() => {
-    const listener = (event: ExecutorEvent) => {
-      setPending(
-        Array.from(manager)
-          .filter(predicate)
-          .some(executor => executor.isPending)
-      );
+    const syncIsPending = (event: ExecutorEvent) => {
+      setPending(Array.from(manager).some(executor => predicate(executor) && executor.isPending));
     };
 
     // 1️⃣ Ensure isPending is up-to-date after mount
-    listener();
+    syncIsPending();
 
     // 2️⃣ Sync isPending when any event is published
-    return manager.subscribe(listener);
+    return manager.subscribe(syncIsPending);
   }, [manager]);
 
   return isPending;
@@ -2242,7 +2238,7 @@ function useGlobalPending(predicate = (executor: Executor) => true): boolean {
 Now a global pending indicator can be shown when _any_ executor is pending:
 
 ```tsx
-const isPending = useGlobalPending();
+const isPending = useIsPending();
 
 isPending && <LoadingIndicator />;
 ```
@@ -2254,17 +2250,14 @@ be marked as such, for example with an annotation:
 const accountExecutor = useExecutor(
   'account',
 
-  async () => {
-    const response = await fetch('/account');
-    return response.json();
-  },
+  () => fetch('/account'),
 
   // 1️⃣ Annotate an executor once via a plugin
   [executor => executor.annotate({ isFetching: true })]
 );
 
 // 2️⃣ Get global pending status for executors that are fetching data
-const isPending = useGlobalPending(executor => executor.annotations.isFetching);
+const isPending = useIsPending(executor => executor.annotations.isFetching);
 ```
 
 <!--/ARTICLE-->

src/main/useExecutorSubscription.ts

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import type { Executor } from './types.js';
+import type { Executor, ExecutorState } from './types.js';
 
 /**
  * Re-renders the component when an executor's state is changed.
@@ -12,30 +12,23 @@ export function useExecutorSubscription<Value>(executor: Executor<Value>): Execu
   React.useDebugValue(executor, getExecutorStateSnapshot);
 
   if (typeof React.useSyncExternalStore === 'function') {
-    const subscribe = React.useCallback(executor.subscribe.bind(executor), [executor]);
+    const getSnapshot = () => getExecutorId(executor) + '.' + executor.version;
 
-    const getSnapshot = () => executor.version;
+    React.useSyncExternalStore(executor.subscribe, getSnapshot, getSnapshot);
 
-    React.useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
-
-    React.useEffect(executor.activate.bind(executor), [executor]);
+    React.useEffect(executor.activate, [executor]);
 
     return executor;
   }
 
   const [, setVersion] = React.useState(executor.version);
 
   React.useEffect(() => {
-    let version = executor.version;
-
     const deactivate = executor.activate();
-    const unsubscribe = executor.subscribe(() => {
-      if (version < executor.version) {
-        setVersion((version = executor.version));
-      }
-    });
 
-    setVersion(version);
+    const unsubscribe = executor.subscribe(() => setVersion(executor.version));
+
+    setVersion(executor.version);
 
     return () => {
       unsubscribe();
@@ -46,6 +39,14 @@ export function useExecutorSubscription<Value>(executor: Executor<Value>): Execu
   return executor;
 }
 
-function getExecutorStateSnapshot(executor: Executor) {
+const executorIds = new WeakMap<Executor, number>();
+
+let executorCount = 0;
+
+function getExecutorId(executor: Executor): number {
+  return executorIds.get(executor) || (executorIds.set(executor, ++executorCount), executorCount);
+}
+
+function getExecutorStateSnapshot(executor: Executor): ExecutorState {
   return executor.getStateSnapshot();
 }

src/test/useExecutor.test.tsx

@@ -3,14 +3,18 @@
  */
 
 import { beforeEach, expect, test, vi } from 'vitest';
-import { act, renderHook } from '@testing-library/react';
+import { act, render, renderHook } from '@testing-library/react';
 import React, { StrictMode } from 'react';
-import { ExecutorManager, ExecutorManagerProvider, useExecutor } from '../main/index.js';
+import { ExecutorEvent, ExecutorManager, ExecutorManagerProvider, useExecutor } from '../main/index.js';
 
 let testIndex = 0;
 let executorKey: string;
 
+vi.useFakeTimers();
+
 beforeEach(() => {
+  vi.clearAllTimers();
+
   executorKey = 'executor' + testIndex++;
 });
 
@@ -165,3 +169,88 @@ test('re-renders after task execute', async () => {
 
   expect(renderMock).toHaveBeenCalledTimes(6);
 });
+
+test('deactivates an executor when key changes after parent component render', async () => {
+  const task = vi.fn();
+  const listenerMock = vi.fn();
+
+  const manager = new ExecutorManager();
+
+  const Parent = (props: { executorKey: string }) => (
+    <ExecutorManagerProvider value={manager}>
+      <Child executorKey={props.executorKey} />
+    </ExecutorManagerProvider>
+  );
+
+  const Child = (props: { executorKey: string }) => {
+    useExecutor(props.executorKey, task, [executor => executor.subscribe(listenerMock)]);
+    return null;
+  };
+
+  const { rerender } = await act(() => render(<Parent executorKey={'xxx'} />));
+
+  expect(manager['_executors'].size).toBe(1);
+
+  expect(manager.get('xxx')!.isActive).toBe(true);
+
+  expect(listenerMock).toHaveBeenCalledTimes(4);
+  expect(listenerMock).toHaveBeenNthCalledWith(1, {
+    type: 'attached',
+    target: manager.get('xxx')!,
+    version: 0,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+  expect(listenerMock).toHaveBeenNthCalledWith(2, {
+    type: 'pending',
+    target: manager.get('xxx')!,
+    version: 1,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+  expect(listenerMock).toHaveBeenNthCalledWith(3, {
+    type: 'activated',
+    target: manager.get('xxx')!,
+    version: 1,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+  expect(listenerMock).toHaveBeenNthCalledWith(4, {
+    type: 'fulfilled',
+    target: manager.get('xxx')!,
+    version: 2,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+
+  act(() => rerender(<Parent executorKey={'yyy'} />));
+
+  vi.runAllTimers();
+
+  expect(manager['_executors'].size).toBe(2);
+
+  expect(manager.get('xxx')!.isActive).toBe(false);
+  expect(manager.get('yyy')!.isActive).toBe(true);
+
+  expect(listenerMock).toHaveBeenCalledTimes(8);
+  expect(listenerMock).toHaveBeenNthCalledWith(5, {
+    type: 'attached',
+    target: manager.get('yyy')!,
+    version: 0,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+  expect(listenerMock).toHaveBeenNthCalledWith(6, {
+    type: 'pending',
+    target: manager.get('yyy')!,
+    version: 1,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+  expect(listenerMock).toHaveBeenNthCalledWith(7, {
+    type: 'deactivated',
+    target: expect.objectContaining({ key: 'xxx' }),
+    version: 2,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+  expect(listenerMock).toHaveBeenNthCalledWith(8, {
+    type: 'activated',
+    target: manager.get('yyy')!,
+    version: 1,
+    payload: undefined,
+  } satisfies ExecutorEvent);
+});