From 14f5f065759759d78849b979f98b294c8a906c5b Mon Sep 17 00:00:00 2001 From: jungwoo3490 Date: Sat, 30 May 2026 02:06:08 +0900 Subject: [PATCH 1/3] feat(relay-runtime): add onSettled callback to commitMutation --- .../__tests__/commitMutation-test.js | 101 ++++++++++++++++++ .../mutations/commitMutation.d.ts | 5 + .../relay-runtime/mutations/commitMutation.js | 29 +++-- 3 files changed, 126 insertions(+), 9 deletions(-) diff --git a/packages/relay-runtime/mutations/__tests__/commitMutation-test.js b/packages/relay-runtime/mutations/__tests__/commitMutation-test.js index c70ba40b146f1..fdf9c07b6456d 100644 --- a/packages/relay-runtime/mutations/__tests__/commitMutation-test.js +++ b/packages/relay-runtime/mutations/__tests__/commitMutation-test.js @@ -1523,6 +1523,107 @@ describe('commitMutation()', () => { expect(onError).toBeCalledTimes(1); expect(onError.mock.calls[0][0]).toBe(error); }); + + it('calls onSettled after onCompleted on success', () => { + /* $FlowFixMe[underconstrained-implicit-instantiation] error found when + * enabling Flow LTI mode */ + const onSettled = jest.fn<_, void>(); + commitMutation(environment, { + mutation, + onCompleted, + onError, + onSettled, + variables, + }); + dataSource.next({ + data: { + commentCreate: { + comment: {body: {text: 'Gave Relay'}, id: '1'}, + }, + }, + }); + dataSource.complete(); + + expect(onCompleted).toBeCalledTimes(1); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled.mock.calls[0][0]).toEqual(onCompleted.mock.calls[0][0]); + expect(onSettled.mock.calls[0][1]).toBe(null); + expect(onSettled.mock.calls[0][2]).toBe(null); + expect(onError).toBeCalledTimes(0); + }); + + it('calls onSettled with payload errors on success with errors', () => { + /* $FlowFixMe[underconstrained-implicit-instantiation] error found when + * enabling Flow LTI mode */ + const onSettled = jest.fn<_, void>(); + commitMutation(environment, { + mutation, + onCompleted, + onError, + onSettled, + variables, + }); + dataSource.next({ + data: { + commentCreate: { + comment: {body: {text: 'Gave Relay'}, id: '1'}, + }, + }, + errors: [{locations: [], message: 'wtf', severity: 'ERROR'}], + } as GraphQLResponseWithoutData); + dataSource.complete(); + + expect(onCompleted).toBeCalledTimes(1); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled.mock.calls[0][0]).toEqual(onCompleted.mock.calls[0][0]); + expect(onSettled.mock.calls[0][1]).toEqual(onCompleted.mock.calls[0][1]); + expect(onSettled.mock.calls[0][2]).toBe(null); + expect(onError).toBeCalledTimes(0); + }); + + it('calls onSettled after onError on network error', () => { + /* $FlowFixMe[underconstrained-implicit-instantiation] error found when + * enabling Flow LTI mode */ + const onSettled = jest.fn<_, void>(); + const error = new Error('network failure'); + commitMutation(environment, { + mutation, + onCompleted, + onError, + onSettled, + variables, + }); + dataSource.error(error); + + expect(onError).toBeCalledTimes(1); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled.mock.calls[0][0]).toBe(null); + expect(onSettled.mock.calls[0][1]).toBe(null); + expect(onSettled.mock.calls[0][2]).toBe(error); + expect(onCompleted).toBeCalledTimes(0); + }); + + it('calls onSettled even without onCompleted or onError', () => { + /* $FlowFixMe[underconstrained-implicit-instantiation] error found when + * enabling Flow LTI mode */ + const onSettled = jest.fn<_, void>(); + commitMutation(environment, { + mutation, + onSettled, + variables, + }); + dataSource.next({ + data: { + commentCreate: { + comment: {body: {text: 'Gave Relay'}, id: '1'}, + }, + }, + }); + dataSource.complete(); + + expect(onSettled).toBeCalledTimes(1); + expect(onSettled.mock.calls[0][2]).toBe(null); + }); }); describe('commitMutation() cacheConfig', () => { diff --git a/packages/relay-runtime/mutations/commitMutation.d.ts b/packages/relay-runtime/mutations/commitMutation.d.ts index c61d5e12c98ad..8437c5d7049e2 100644 --- a/packages/relay-runtime/mutations/commitMutation.d.ts +++ b/packages/relay-runtime/mutations/commitMutation.d.ts @@ -26,6 +26,11 @@ export interface MutationConfig { | ((response: TOperation['response'], errors: readonly PayloadError[] | null | undefined) => void) | null | undefined; + onSettled?: (( + response: TOperation['response'] | null, + errors: readonly PayloadError[] | null | undefined, + error: Error | null | undefined, + ) => void) | null | undefined; onUnsubscribe?: (() => void | null | undefined) | undefined; /** * An object whose type matches the raw response type of the mutation. Make sure you decorate diff --git a/packages/relay-runtime/mutations/commitMutation.js b/packages/relay-runtime/mutations/commitMutation.js index 97853ccef719e..baa6ce5bfa966 100644 --- a/packages/relay-runtime/mutations/commitMutation.js +++ b/packages/relay-runtime/mutations/commitMutation.js @@ -47,6 +47,11 @@ export type MutationConfig = Readonly<{ ) => void, onError?: ?(error: Error) => void, onNext?: ?() => void, + onSettled?: ?( + response: TMutation['response'] | null, + errors: ?Array, + error: ?Error, + ) => void, onUnsubscribe?: ?() => void, optimisticResponse?: { readonly rawResponse?: {...}, @@ -66,6 +71,11 @@ export type CommitMutationConfig = Readonly<{ onCompleted?: ?(response: TData, errors: ?Array) => void, onError?: ?(error: Error) => void, onNext?: ?() => void, + onSettled?: ?( + response: TData | null, + errors: ?Array, + error: ?Error, + ) => void, onUnsubscribe?: ?() => void, optimisticResponse?: TRawResponse, optimisticUpdater?: ?SelectorStoreUpdater, @@ -99,8 +109,7 @@ function commitMutation< throw new Error('commitMutation: Expected mutation to be of type request'); } let {optimisticResponse, optimisticUpdater, updater} = config; - const {configs, cacheConfig, onError, onUnsubscribe, variables, uploadables} = - config; + const {configs, cacheConfig, onUnsubscribe, variables, uploadables} = config; const operation = createOperationDescriptor( mutation, variables, @@ -147,16 +156,18 @@ function commitMutation< }) .subscribe({ complete: () => { - const {onCompleted} = config; - if (onCompleted) { + const {onCompleted, onSettled} = config; + const payloadErrors = errors.length !== 0 ? errors : null; + if (onCompleted != null || onSettled != null) { const snapshot = environment.lookup(operation.fragment); - onCompleted( - snapshot.data as $FlowFixMe, - errors.length !== 0 ? errors : null, - ); + onCompleted?.(snapshot.data as $FlowFixMe, payloadErrors); + onSettled?.(snapshot.data as $FlowFixMe, payloadErrors, null); } }, - error: onError, + error: (err: Error) => { + config.onError?.(err); + config.onSettled?.(null, null, err); + }, next: payload => { if (Array.isArray(payload)) { payload.forEach(item => { From 0066f5162717cbf64d57d379ff77ef3b777eca68 Mon Sep 17 00:00:00 2001 From: jungwoo3490 Date: Sat, 30 May 2026 20:38:53 +0900 Subject: [PATCH 2/3] refactor(relay-runtime): unify destructuring pattern in commitMutation handlers --- .../__tests__/commitMutation-test.js | 29 ++++++++++++++++--- .../relay-runtime/mutations/commitMutation.js | 5 ++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/relay-runtime/mutations/__tests__/commitMutation-test.js b/packages/relay-runtime/mutations/__tests__/commitMutation-test.js index fdf9c07b6456d..4b767d40741d3 100644 --- a/packages/relay-runtime/mutations/__tests__/commitMutation-test.js +++ b/packages/relay-runtime/mutations/__tests__/commitMutation-test.js @@ -1538,7 +1538,12 @@ describe('commitMutation()', () => { dataSource.next({ data: { commentCreate: { - comment: {body: {text: 'Gave Relay'}, id: '1'}, + comment: { + body: { + text: 'Gave Relay', + }, + id: '1', + }, }, }, }); @@ -1566,10 +1571,21 @@ describe('commitMutation()', () => { dataSource.next({ data: { commentCreate: { - comment: {body: {text: 'Gave Relay'}, id: '1'}, + comment: { + body: { + text: 'Gave Relay', + }, + id: '1', + }, }, }, - errors: [{locations: [], message: 'wtf', severity: 'ERROR'}], + errors: [ + { + locations: [], + message: 'wtf', + severity: 'ERROR', + }, + ], } as GraphQLResponseWithoutData); dataSource.complete(); @@ -1615,7 +1631,12 @@ describe('commitMutation()', () => { dataSource.next({ data: { commentCreate: { - comment: {body: {text: 'Gave Relay'}, id: '1'}, + comment: { + body: { + text: 'Gave Relay', + }, + id: '1', + }, }, }, }); diff --git a/packages/relay-runtime/mutations/commitMutation.js b/packages/relay-runtime/mutations/commitMutation.js index baa6ce5bfa966..3ff712079bb1b 100644 --- a/packages/relay-runtime/mutations/commitMutation.js +++ b/packages/relay-runtime/mutations/commitMutation.js @@ -165,8 +165,9 @@ function commitMutation< } }, error: (err: Error) => { - config.onError?.(err); - config.onSettled?.(null, null, err); + const {onError, onSettled} = config; + onError?.(err); + onSettled?.(null, null, err); }, next: payload => { if (Array.isArray(payload)) { From c7c3300858343289dc9e652ec28c3d18a5cc6a97 Mon Sep 17 00:00:00 2001 From: jungwoo3490 Date: Sat, 30 May 2026 20:40:00 +0900 Subject: [PATCH 3/3] feat(react-relay): add onSettled callback to useMutation --- .../relay-hooks/__tests__/useMutation-test.js | 90 +++++++++++++++++++ .../react-relay/relay-hooks/useMutation.d.ts | 5 ++ .../react-relay/relay-hooks/useMutation.js | 6 ++ 3 files changed, 101 insertions(+) diff --git a/packages/react-relay/relay-hooks/__tests__/useMutation-test.js b/packages/react-relay/relay-hooks/__tests__/useMutation-test.js index 6abed6d299768..5b1cc010e7f78 100644 --- a/packages/react-relay/relay-hooks/__tests__/useMutation-test.js +++ b/packages/react-relay/relay-hooks/__tests__/useMutation-test.js @@ -434,6 +434,96 @@ it('calls onComplete when mutation successfully resolved', async () => { expect(isInFlightFn).toBeCalledWith(false); }); +it('calls onSettled after onCompleted on success', async () => { + const onError = jest.fn, unknown>(); + const onCompleted = jest.fn, unknown>(); + const onSettled = jest.fn, unknown>(); + await render(environment, CommentCreateMutation); + await commit({onCompleted, onError, onSettled, variables}); + + isInFlightFn.mockClear(); + // $FlowFixMe[method-unbinding] added when improving typing for this parameters + const operation = environment.executeMutation.mock.calls[0][0].operation; + await ReactTestingLibrary.act(() => + environment.mock.resolve(operation, data), + ); + expect(onCompleted).toBeCalledTimes(1); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled).toBeCalledWith( + onCompleted.mock.calls[0][0], + null, + null, + ); + expect(onError).toBeCalledTimes(0); + expect(isInFlightFn).toBeCalledWith(false); +}); + +it('calls onSettled with payload errors on success with errors', async () => { + const onError = jest.fn, unknown>(); + const onCompleted = jest.fn, unknown>(); + const onSettled = jest.fn, unknown>(); + await render(environment, CommentCreateMutation); + await commit({onCompleted, onError, onSettled, variables}); + + isInFlightFn.mockClear(); + // $FlowFixMe[method-unbinding] added when improving typing for this parameters + const operation = environment.executeMutation.mock.calls[0][0].operation; + await ReactTestingLibrary.act(() => + environment.mock.resolve(operation, { + data: data.data as PayloadData, + errors: [{message: ''}] as Array, + }), + ); + expect(onCompleted).toBeCalledTimes(1); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled).toBeCalledWith( + onCompleted.mock.calls[0][0], + [{message: ''}], + null, + ); + expect(onError).toBeCalledTimes(0); + expect(isInFlightFn).toBeCalledWith(false); +}); + +it('calls onSettled after onError on error', async () => { + const onError = jest.fn, unknown>(); + const onCompleted = jest.fn, unknown>(); + const onSettled = jest.fn, unknown>(); + const throwingUpdater = () => { + throw new Error(''); + }; + await render(environment, CommentCreateMutation); + await commit({onCompleted, onError, onSettled, updater: throwingUpdater, variables}); + + isInFlightFn.mockClear(); + // $FlowFixMe[method-unbinding] added when improving typing for this parameters + const operation = environment.executeMutation.mock.calls[0][0].operation; + await ReactTestingLibrary.act(() => + environment.mock.resolve(operation, data), + ); + expect(onError).toBeCalledTimes(1); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled).toBeCalledWith(null, null, new Error('')); + expect(onCompleted).toBeCalledTimes(0); + expect(isInFlightFn).toBeCalledWith(false); +}); + +it('calls onSettled even without onCompleted or onError', async () => { + const onSettled = jest.fn, unknown>(); + await render(environment, CommentCreateMutation); + await commit({onSettled, variables}); + + isInFlightFn.mockClear(); + // $FlowFixMe[method-unbinding] added when improving typing for this parameters + const operation = environment.executeMutation.mock.calls[0][0].operation; + await ReactTestingLibrary.act(() => + environment.mock.resolve(operation, data), + ); + expect(onSettled).toBeCalledTimes(1); + expect(onSettled.mock.calls[0][2]).toBe(null); + expect(isInFlightFn).toBeCalledWith(false); +}); + describe('change useMutation input', () => { let newEnv; let CommentCreateMutation2; diff --git a/packages/react-relay/relay-hooks/useMutation.d.ts b/packages/react-relay/relay-hooks/useMutation.d.ts index 79ad37c940f8a..1828457f9983d 100644 --- a/packages/react-relay/relay-hooks/useMutation.d.ts +++ b/packages/react-relay/relay-hooks/useMutation.d.ts @@ -27,6 +27,11 @@ export interface UseMutationConfig { configs?: DeclarativeMutationConfig[] | undefined; onError?: ((error: Error) => void | null) | undefined; onCompleted?: ((response: TMutation['response'], errors: PayloadError[] | null) => void | null) | undefined; + onSettled?: (( + response: TMutation['response'] | null, + errors: PayloadError[] | null | undefined, + error: Error | null | undefined, + ) => void | null) | undefined; onUnsubscribe?: (() => void | null) | undefined; } diff --git a/packages/react-relay/relay-hooks/useMutation.js b/packages/react-relay/relay-hooks/useMutation.js index 8a3bf593a848b..1a3398a6a173b 100644 --- a/packages/react-relay/relay-hooks/useMutation.js +++ b/packages/react-relay/relay-hooks/useMutation.js @@ -38,6 +38,11 @@ export type UseMutationConfig = { errors: ?Array, ) => void, onNext?: ?() => void, + onSettled?: ?( + response: TMutation['response'] | null, + errors: ?Array, + error: ?Error, + ) => void, onUnsubscribe?: ?() => void, optimisticResponse?: { readonly rawResponse?: {...}, @@ -55,6 +60,7 @@ type UseMutationConfigInternal = { onError?: ?(error: Error) => void, onCompleted?: ?(response: TData, errors: ?Array) => void, onNext?: ?() => void, + onSettled?: ?(response: TData | null, errors: ?Array, error: ?Error) => void, onUnsubscribe?: ?() => void, optimisticResponse?: TRawResponse, optimisticUpdater?: ?SelectorStoreUpdater,