diff --git a/.changeset/every-plums-train.md b/.changeset/every-plums-train.md new file mode 100644 index 0000000..4cde210 --- /dev/null +++ b/.changeset/every-plums-train.md @@ -0,0 +1,6 @@ +--- +"@connectivity-js/react": minor +"@connectivity-js/core": minor +--- + +`client.execute()` previously accepted only a string `actionKey`, causing the diff --git a/.changeset/yellow-facts-tan.md b/.changeset/yellow-facts-tan.md new file mode 100644 index 0000000..d750dbe --- /dev/null +++ b/.changeset/yellow-facts-tan.md @@ -0,0 +1,6 @@ +--- +"@connectivity-js/devtools": minor +"@connectivity-js/react-devtools": minor +--- + +version bump diff --git a/packages/core/src/action-observer.ts b/packages/core/src/action-observer.ts index c06187a..ee9f25e 100644 --- a/packages/core/src/action-observer.ts +++ b/packages/core/src/action-observer.ts @@ -97,6 +97,19 @@ export class ActionObserver { /** * Executes the action and handles callbacks. + * + * Uses the string overload of `client.execute()` to ensure the Map's + * registered request is always used — consistent with the flush queue path. + * `engineResult.result` is `unknown` from the string overload, so a + * cast to `TResult` is applied. + * + * **Safety condition:** The cast is safe as long as the Map entry for + * `this.#options.actionKey` returns a value compatible with `TResult`. + * In normal `useAction` usage this is always true because `setOptions()` + * re-runs `#register()` on every render, keeping the Map in sync with the + * declared `TResult`. A manual `registerAction()` override with a different + * return type between renders would make the cast technically incorrect until + * the next `setOptions()` call restores the correct registration. */ async execute(input: TInput) { let wasEnqueued = false; diff --git a/packages/core/src/connectivity-client.ts b/packages/core/src/connectivity-client.ts index b320c07..fdc891f 100644 --- a/packages/core/src/connectivity-client.ts +++ b/packages/core/src/connectivity-client.ts @@ -1,3 +1,4 @@ +import type { ActionOptionsConfig } from './action-options'; import { DEFAULT_BACKOFF_MS, delay, @@ -437,24 +438,53 @@ export class ConnectivityClient { /** * Executes a registered action. * + * **Overload 1 — Type-safe with `ActionOptionsConfig`:** + * Pass an `actionOptions()` config to get fully inferred `TInput` and `TResult`. + * Auto-registers the action if it has not been registered yet. + * + * **Overload 2 — String key (backward compatible):** + * Pass a previously registered action key. Result type is `unknown`. + * * - **online**: Executes immediately and returns the result (`{ enqueued: false, result }`) * - **offline + `whenOffline: 'queue'`**: Queues the request (`{ enqueued: true, jobId }`) * - **offline + `whenOffline: 'fail'`**: Throws an error immediately * - * @param actionKey - Unique key of the action to execute - * @param input - Input value to pass to the action - * @returns Discriminated union result * @throws When action is not registered or when executing offline with `whenOffline: 'fail'` * * @example - * const result = await client.execute('save', { id: '1', data: 'hello' }); - * if (result.enqueued) { - * console.log('Queued:', result.jobId); - * } else { - * console.log('Executed immediately:', result.result); + * // Type-safe with actionOptions + * const saveAction = actionOptions({ + * actionKey: 'save', + * request: (input: { id: string; data: string }) => api.save(input), + * }); + * const result = await client.execute(saveAction, { id: '1', data: 'hello' }); + * if (!result.enqueued) { + * result.result; // fully typed as the return of api.save() * } + * + * @example + * // String key (backward compatible) + * const result = await client.execute('save', { id: '1', data: 'hello' }); */ - async execute(actionKey: string, input: unknown) { + async execute( + config: ActionOptionsConfig, + input: TInput, + ): Promise>; + async execute(actionKey: string, input: unknown): Promise; + async execute( + configOrKey: string | ActionOptionsConfig, + input: TInput, + ): Promise> { + const actionKey = + typeof configOrKey === 'string' ? configOrKey : configOrKey.actionKey; + + // Auto-register when called with config and the action is not yet registered. + // If already registered (e.g. by ActionObserver with flush callbacks), the + // existing registration is preserved so that flush callbacks are not lost. + if (typeof configOrKey !== 'string' && !this.#actions.has(actionKey)) { + this.#registerFromConfig(configOrKey); + } + const action = this.#actions.get(actionKey); if (action === undefined) { throw new Error( @@ -487,7 +517,7 @@ export class ConnectivityClient { // Immediate execution only when confirmed online — if unknown, queue and flush after detector emits if (!isConfirmedOnline) { - return { enqueued: true as const, jobId } satisfies ActionRunResult; + return { enqueued: true as const, jobId }; } const hasRunningDupe = @@ -500,7 +530,7 @@ export class ConnectivityClient { j.status === 'running', ); if (hasRunningDupe) { - return { enqueued: true as const, jobId } satisfies ActionRunResult; + return { enqueued: true as const, jobId }; } this.#queuePatch(jobId, { @@ -510,15 +540,22 @@ export class ConnectivityClient { this.#notifyQueue(); try { + // Config path: call config.request directly — TResult is fully inferred, + // no type assertion needed. The Map is not involved in the request call or + // result typing; however, mergedOptions (whenOffline, retry) still comes from + // the Map's registered action.options (set above via auto-register or prior + // registerAction call). + // String path: call action.request from Map — result is unknown, + // TResult defaults to unknown so ActionRunResult is returned. + if (typeof configOrKey !== 'string') { + const result = await configOrKey.request(input); + this.#onImmediateSuccess(jobId); + return { enqueued: false as const, result }; + } + const result = await action.request(input); - this.#queuePatch(jobId, { status: 'succeeded' }); - this.#notifyQueue(); - this.#scheduleTimer(() => { - this.#queueRemove(jobId); - this.#notifyQueue(); - }, SUCCEEDED_JOB_CLEANUP_DELAY_MS); - void this.#flushQueue(); - return { enqueued: false as const, result } satisfies ActionRunResult; + this.#onImmediateSuccess(jobId); + return { enqueued: false as const, result: result as TResult }; } catch (error: unknown) { return this.#resolveExecuteFailure({ error, @@ -530,6 +567,35 @@ export class ConnectivityClient { } } + #onImmediateSuccess(jobId: string) { + this.#queuePatch(jobId, { status: 'succeeded' }); + this.#notifyQueue(); + this.#scheduleTimer(() => { + this.#queueRemove(jobId); + this.#notifyQueue(); + }, SUCCEEDED_JOB_CLEANUP_DELAY_MS); + void this.#flushQueue(); + } + + #registerFromConfig( + config: ActionOptionsConfig, + ) { + const { dedupeKey } = config; + this.registerAction(config.actionKey, { + request: (input) => config.request(input as TInput), + options: { + whenOffline: config.whenOffline, + retry: config.retry, + flushOption: config.flushOption, + dedupeKey: + dedupeKey !== undefined + ? (input) => dedupeKey(input as TInput) + : undefined, + dedupeOnFlush: config.dedupeOnFlush, + }, + }); + } + #enqueueJob(actionKey: string, input: unknown) { const action = this.#actions.get(actionKey); const dedupeKey = action?.options.dedupeKey?.(input); @@ -610,7 +676,7 @@ export class ConnectivityClient { }); this.#notifyQueue(); void this.#flushQueue(); - return { enqueued: true as const, jobId } satisfies ActionRunResult; + return { enqueued: true as const, jobId }; } const currentAttempt = (job?.attempt ?? 0) + 1; @@ -624,7 +690,7 @@ export class ConnectivityClient { this.#notifyQueue(); this.#scheduleRetry(jobId, backoffMs); void this.#flushQueue(); - return { enqueued: true as const, jobId } satisfies ActionRunResult; + return { enqueued: true as const, jobId }; } // ─── Queue Control ─────────────────────────── diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index af5b9c8..334afc7 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -156,6 +156,8 @@ export interface QueuedJob { /** * Discriminated union returned by `execute()` * + * @typeParam TResult - Result type returned by the action (default: `unknown`) + * * @example * const result = await client.execute('save', input); * if (result.enqueued) { @@ -164,9 +166,9 @@ export interface QueuedJob { * console.log('executed immediately:', result.result); * } */ -export type ActionRunResult = +export type ActionRunResult = | { enqueued: true; jobId: string } - | { enqueued: false; result: unknown }; + | { enqueued: false; result: TResult }; /** * Options passed when creating a {@link ConnectivityClient} instance diff --git a/packages/core/tests/action-observer.test.ts b/packages/core/tests/action-observer.test.ts index a6978d9..48e9bdd 100644 --- a/packages/core/tests/action-observer.test.ts +++ b/packages/core/tests/action-observer.test.ts @@ -236,6 +236,36 @@ describe('ActionObserver', () => { expect(result1).toBe(result2); }); + test('registerAction after observer creation overrides the request for execute', async () => { + const mock = createMockDetector(); + const client = getConnectivityClient({ detectors: [mock.detector] }); + client.start(); + mock.emit({ status: 'online', reason: 'test' }); + + const original = vi.fn().mockResolvedValue('original'); + const override = vi.fn().mockResolvedValue('override'); + + const observer = new ActionObserver(client, { + actionKey: 'save', + request: original, + }); + + // Override the registration via client.registerAction after the observer was created. + // NOTE: this override also replaces the onFlushSuccess/onFlushError/onFlushSettled + // callbacks that ActionObserver had registered. As a result, when a queued offline + // job is later flushed, those callbacks will NOT be invoked. + // In a useAction context this is transient — the next render calls setOptions() → + // #register(), which re-registers the observer's own callbacks. + client.registerAction('save', { request: override, options: {} }); + + const r = await observer.execute({}); + + // The Map override is used — original is never called + expect(original).not.toHaveBeenCalled(); + expect(override).toHaveBeenCalledOnce(); + expect(r).toMatchObject({ enqueued: false, result: 'override' }); + }); + test('updating callback via setCallbacks invokes the latest callback', async () => { const mock = createMockDetector(); const client = getConnectivityClient({ detectors: [mock.detector] }); diff --git a/packages/core/tests/connectivity-client.test-d.ts b/packages/core/tests/connectivity-client.test-d.ts index 6c2cef1..7ae87b2 100644 --- a/packages/core/tests/connectivity-client.test-d.ts +++ b/packages/core/tests/connectivity-client.test-d.ts @@ -1,4 +1,5 @@ import { describe, expectTypeOf, it } from 'vitest'; +import { actionOptions } from '../src/action-options'; import { getConnectivityClient } from '../src/connectivity-client'; import type { ActionRunResult, @@ -52,6 +53,49 @@ describe('ConnectivityClient.execute', () => { }); }); +describe('ConnectivityClient.execute with config overload — type inference', () => { + const client = getConnectivityClient(); + + const saveAction = actionOptions({ + actionKey: 'save', + request: async (_input: { id: string; data: string }) => ({ + saved: true as const, + timestamp: 123, + }), + }); + + it('result is narrowed to TResult in enqueued === false branch', async () => { + const result = await client.execute(saveAction, { + id: '1', + data: 'hello', + }); + if (!result.enqueued) { + expectTypeOf(result.result).toExtend<{ + saved: true; + timestamp: number; + }>(); + } + }); + + it('input type is enforced as TInput', () => { + // @ts-expect-error — must pass { id: string; data: string } but passing number + client.execute(saveAction, 42); + }); + + it('string key overload result stays unknown (backward compatible)', async () => { + const result = await client.execute('save', {}); + if (!result.enqueued) { + expectTypeOf(result.result).toBeUnknown(); + } + }); + + it('ActionRunResult generic narrows result correctly', () => { + type Typed = ActionRunResult<{ id: string }>; + type Extracted = Extract['result']; + expectTypeOf().toExtend<{ id: string }>(); + }); +}); + describe('ConnectivityClient.subscribe', () => { const client = getConnectivityClient(); diff --git a/packages/core/tests/connectivity-client.test.ts b/packages/core/tests/connectivity-client.test.ts index e9bdeaa..ad43302 100644 --- a/packages/core/tests/connectivity-client.test.ts +++ b/packages/core/tests/connectivity-client.test.ts @@ -7,6 +7,7 @@ import { test, vi, } from 'vitest'; +import { actionOptions } from '../src/action-options'; import { ConnectivityClient, getConnectivityClient, @@ -1367,4 +1368,126 @@ describe('ConnectivityClient', () => { expect(onFlushSettled).toHaveBeenCalledOnce(); }); }); + + describe('execute with config overload', () => { + test('online → immediate result (same behavior as string key)', async () => { + const { client, mock } = createTestClient(); + mock.emit({ status: 'online', reason: 'test' }); + + const saveAction = actionOptions({ + actionKey: 'save-cfg', + request: async (input: { id: string; data: string }) => ({ + saved: true, + id: input.id, + }), + }); + + const r = await client.execute(saveAction, { + id: '1', + data: 'hello', + }); + expect(r.enqueued).toBe(false); + if (!r.enqueued) { + expect(r.result).toEqual({ saved: true, id: '1' }); + } + }); + + test('auto-registers when action is not registered', async () => { + const { client, mock } = createTestClient(); + mock.emit({ status: 'online', reason: 'test' }); + + const action = actionOptions({ + actionKey: 'auto-reg', + request: async () => 'ok', + }); + + // No registerAction call — should not throw "not registered" + const r = await client.execute(action, undefined); + expect(r.enqueued).toBe(false); + if (!r.enqueued) { + expect(r.result).toBe('ok'); + } + }); + + test('offline → enqueued', async () => { + const { client, mock } = createTestClient(); + mock.emit({ status: 'offline', reason: 'test' }); + + const action = actionOptions({ + actionKey: 'save-cfg-off', + request: async (_input: { id: string }) => ({ saved: true }), + whenOffline: 'queue', + }); + + const r = await client.execute(action, { id: '1' }); + expect(r.enqueued).toBe(true); + }); + + test('dedupeKey works with config overload', async () => { + const { client, mock } = createTestClient(); + mock.emit({ status: 'offline', reason: 'test' }); + + const action = actionOptions({ + actionKey: 'save-dedup', + request: async (_input: { id: string; data: string }) => ({ + saved: true, + }), + whenOffline: 'queue', + dedupeKey: (input) => input.id, + }); + + const r1 = await client.execute(action, { id: 'a', data: 'v1' }); + const r2 = await client.execute(action, { id: 'a', data: 'v2' }); + if (r1.enqueued && r2.enqueued) { + expect(r1.jobId).toBe(r2.jobId); + } + const job = client.getQueue()[0]; + assert(job !== undefined); + expect(job.input).toEqual({ id: 'a', data: 'v2' }); + }); + + test('preserves existing registration with flush callbacks', async () => { + const { client, mock } = createTestClient(); + mock.emit({ status: 'offline', reason: 'test' }); + + const onFlushSuccess = vi.fn(); + + // Register manually with flush callbacks first + client.registerAction('save-preserve', { + request: vi.fn().mockResolvedValue({ saved: true }), + options: { whenOffline: 'queue' }, + onFlushSuccess, + }); + + // Config overload with same actionKey — should NOT overwrite registration + const action = actionOptions({ + actionKey: 'save-preserve', + request: async () => ({ saved: true }), + whenOffline: 'queue', + }); + + await client.execute(action, {}); + + mock.emit({ status: 'online', reason: 'test' }); + await vi.advanceTimersByTimeAsync(0); + + // Flush callback from registerAction should still be called + expect(onFlushSuccess).toHaveBeenCalledWith({ saved: true }); + }); + + test('whenOffline: fail with config overload', async () => { + const { client, mock } = createTestClient(); + mock.emit({ status: 'offline', reason: 'test' }); + + const action = actionOptions({ + actionKey: 'fail-cfg', + request: async () => 'ok', + whenOffline: 'fail', + }); + + await expect(client.execute(action, undefined)).rejects.toThrow( + "whenOffline='fail'", + ); + }); + }); }); diff --git a/packages/react/tests/use-action.test.tsx b/packages/react/tests/use-action.test.tsx index f87af4e..c7731c8 100644 --- a/packages/react/tests/use-action.test.tsx +++ b/packages/react/tests/use-action.test.tsx @@ -144,6 +144,7 @@ describe('useAction', () => { wrapper: Wrapper, }); + // Override with registerAction after useAction — the override is respected, causing a throw getConnectivityClient().registerAction('purchase', { request: async () => { throw new Error('failed');