Fixed executor subscription

Author: smikhalevski

README.md

@@ -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 { useDebugValue, useEffect, useState } from 'react';
 
 /**
  * Re-renders the component when an executor's state is changed.
@@ -9,33 +9,16 @@ import type { Executor } from './types.js';
  * @param executor The executor to subscribe to.
  */
 export function useExecutorSubscription<Value>(executor: Executor<Value>): Executor<Value> {
-  React.useDebugValue(executor, getExecutorStateSnapshot);
+  const [, setVersion] = useState(executor.version);
 
-  if (typeof React.useSyncExternalStore === 'function') {
-    const subscribe = React.useCallback(executor.subscribe.bind(executor), [executor]);
-
-    const getSnapshot = () => executor.version;
-
-    React.useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
-
-    React.useEffect(executor.activate.bind(executor), [executor]);
-
-    return executor;
-  }
-
-  const [, setVersion] = React.useState(executor.version);
-
-  React.useEffect(() => {
-    let version = executor.version;
+  useDebugValue(executor, getExecutorStateSnapshot);
 
+  useEffect(() => {
     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();

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++;
 });
 
@@ -163,5 +167,90 @@ test('re-renders after task execute', async () => {
   expect(executor.reason).toBeUndefined();
   expect(executor.task).toBe(task);
 
-  expect(renderMock).toHaveBeenCalledTimes(6);
+  expect(renderMock).toHaveBeenCalledTimes(4);
+});
+
+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);
 });