From 24198b92c678f64b27de1ca943604033f356674e Mon Sep 17 00:00:00 2001 From: morrys Date: Fri, 12 Feb 2021 10:33:11 +0100 Subject: [PATCH 1/3] added tests Added relay useMutation tests and adapted to relay-hooks version --- __tests__/useMutation-test.tsx | 431 +++++++++++++++++++++++++++++++++ 1 file changed, 431 insertions(+) create mode 100644 __tests__/useMutation-test.tsx diff --git a/__tests__/useMutation-test.tsx b/__tests__/useMutation-test.tsx new file mode 100644 index 00000000..6da6ee6e --- /dev/null +++ b/__tests__/useMutation-test.tsx @@ -0,0 +1,431 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails oncall+relay + * @flow + * @format + */ + +// flowlint ambiguous-object-type:error + +'use strict'; + +import * as React from 'react'; +import * as ReactTestRenderer from 'react-test-renderer'; + +import { createOperationDescriptor, PayloadData, PayloadError } from 'relay-runtime'; +import { createMockEnvironment, generateAndCompile } from 'relay-test-utils-internal'; +import { useMutation, RelayEnvironmentProvider } from '../src'; + +const { useState, useMemo } = React; +let environment; +let render; +let setEnvironment; +let setMutation; +let commit; +let isInFlightFn; +let CommentCreateMutation; +let instance; +let renderSpy; + +const data = { + data: { + commentCreate: { + feedbackCommentEdge: { + __typename: 'CommentsEdge', + cursor: '', + node: { + id: '', + body: { + text: '', + }, + }, + }, + }, + }, +}; + +const optimisticResponse = { + commentCreate: { + feedbackCommentEdge: { + __typename: 'CommentsEdge', + cursor: '', + node: { + id: '', + body: { + text: 'optimistic ', + }, + }, + }, + }, +}; + +const variables = { + input: { + commentId: '', + }, +}; + +function expectFragmentResult(data, error): void { + // This ensures that useEffect runs + ReactTestRenderer.act(() => jest.runAllImmediates()); + expect(renderSpy).toBeCalledTimes(1); + const [dataRender, errorRender] = renderSpy.mock.calls[0]; + expect(data).toEqual(dataRender); + expect(error).toEqual(errorRender); + renderSpy.mockClear(); +} + +beforeEach(() => { + environment = createMockEnvironment(); + isInFlightFn = jest.fn(); + renderSpy = jest.fn(); + + ({ CommentCreateMutation } = generateAndCompile(` + mutation CommentCreateMutation( + $input: CommentCreateInput + ) { + commentCreate(input: $input) { + feedbackCommentEdge { + cursor + node { + id + body { + text + } + } + } + } + }`)); + + function Renderer({ initialMutation, commitInRender }): any { + const [mutation, setMutationFn] = useState(initialMutation); + setMutation = setMutationFn; + const [commitFn, { loading: isMutationInFlight, data, error }] = useMutation(mutation); + commit = commitFn; + if (commitInRender) { + // `commitInRender` never changes in the test + // eslint-disable-next-line react-hooks/rules-of-hooks + useMemo(() => { + commit({ variables }); + }, []); + } + isInFlightFn(isMutationInFlight); + renderSpy(data, error); + return null; + } + + function Container(props: any): any { + const [env, setEnv] = useState(props.environment); + setEnvironment = setEnv; + return ( + + + + ); + } + + render = function(env, mutation, commitInRender = false): any { + ReactTestRenderer.act(() => { + instance = ReactTestRenderer.create( + , + ); + }); + }; +}); + +it('returns correct in-flight state when the mutation is inflight and completes', () => { + render(environment, CommentCreateMutation); + expect(isInFlightFn).toBeCalledTimes(1); + expect(isInFlightFn).toBeCalledWith(false); + + isInFlightFn.mockClear(); + commit({ variables }); + expect(isInFlightFn).toBeCalledTimes(1); + expect(isInFlightFn).toBeCalledWith(true); + + isInFlightFn.mockClear(); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + expect(isInFlightFn).toBeCalledTimes(1); + expect(isInFlightFn).toBeCalledWith(false); +}); + +it('returns correct in-flight state when commit called inside render', () => { + render(environment, CommentCreateMutation, true); + expect(isInFlightFn).toBeCalledTimes(2); + expect(isInFlightFn).toHaveBeenNthCalledWith(2, true); + isInFlightFn.mockClear(); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + expect(isInFlightFn).toBeCalledTimes(1); + expect(isInFlightFn).toHaveBeenCalledWith(false); +}); + +it('calls onCompleted when mutation responses contains server errors', () => { + const onError = jest.fn(); + const onCompleted = jest.fn(); + render(environment, CommentCreateMutation); + commit({ variables, onError, onCompleted }); + const operation = environment.executeMutation.mock.calls[0][0].operation; + + isInFlightFn.mockClear(); + renderSpy.mockClear(); + ReactTestRenderer.act(() => + environment.mock.resolve(operation, { + data: data.data as PayloadData, + errors: [ + { + message: '', + }, + { + message: '', + }, + ] as Array, + }), + ); + expect(onError).toBeCalledTimes(1); // changed + expect(onCompleted).toBeCalledTimes(0); // changed + const errors = [ + { + message: '', + }, + { + message: '', + }, + ]; + expect(onError).toBeCalledWith(errors); + expect(isInFlightFn).toBeCalledWith(false); + expectFragmentResult(null, errors); +}); +it('calls onError when mutation errors in commitMutation', () => { + const onError = jest.fn(); + const onCompleted = jest.fn(); + const throwingUpdater = (): void => { + throw new Error(''); + }; + render(environment, CommentCreateMutation); + commit({ variables, onError, onCompleted, updater: throwingUpdater }); + + isInFlightFn.mockClear(); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + expect(onError).toBeCalledTimes(1); + expect(onError).toBeCalledWith(new Error('')); + expect(onCompleted).toBeCalledTimes(0); + expect(isInFlightFn).toBeCalledWith(false); +}); + +it('calls onComplete when mutation successfully resolved', () => { + const onError = jest.fn(); + const onCompleted = jest.fn(); + render(environment, CommentCreateMutation); + commit({ variables, onError, onCompleted }); + + isInFlightFn.mockClear(); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + expect(onError).toBeCalledTimes(0); + expect(onCompleted).toBeCalledTimes(1); + expect(onCompleted).toBeCalledWith({ + commentCreate: { + feedbackCommentEdge: { + cursor: '', + node: { + id: '', + body: { + text: '', + }, + }, + }, + }, + }); + expect(isInFlightFn).toBeCalledWith(false); +}); + +describe('change useMutation input', () => { + let newEnv; + let CommentCreateMutation2; + + beforeEach(() => { + newEnv = createMockEnvironment(); + ({ CommentCreateMutation2 } = generateAndCompile(` + mutation CommentCreateMutation2( + $input: CommentCreateInput + ) { + commentCreate(input: $input) { + feedbackCommentEdge { + cursor + node { + id + body { + text + } + } + } + } + }`)); + }); + + it('can fetch from the new environment when the environment changes', () => { + render(environment, CommentCreateMutation); + isInFlightFn.mockClear(); + commit({ variables }); + expect(environment.executeMutation).toBeCalledTimes(1); + + ReactTestRenderer.act(() => setEnvironment(newEnv)); + commit({ variables }); + expect(newEnv.executeMutation).toBeCalledTimes(1); + }); + + it('can fetch use the new query when the query changes', () => { + render(environment, CommentCreateMutation); + commit({ variables }); + + ReactTestRenderer.act(() => setMutation(CommentCreateMutation2)); + commit({ variables }); + const secondOperation = createOperationDescriptor(CommentCreateMutation2, variables); + expect(environment.executeMutation).toBeCalledTimes(2); + expect(environment.executeMutation.mock.calls[1][0].operation.request).toEqual( + secondOperation.request, + ); + + isInFlightFn.mockClear(); + ReactTestRenderer.act(() => { + environment.mock.resolve(environment.executeMutation.mock.calls[0][0].operation, data); + environment.mock.resolve(secondOperation, data); + }); + expect(isInFlightFn).toBeCalledTimes(1); + expect(isInFlightFn).toBeCalledWith(false); + }); +}); + +describe('unmount', () => { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementationOnce(() => {}); + }); + + it('does not setState on commit after unmount', () => { + render(environment, CommentCreateMutation); + ReactTestRenderer.act(() => instance.unmount()); + + isInFlightFn.mockClear(); + commit({ variables }); + expect(isInFlightFn).toBeCalledTimes(0); + expect(console.error).toBeCalledTimes(0); + }); + + it('does not setState on complete after unmount', () => { + render(environment, CommentCreateMutation); + commit({ variables }); + ReactTestRenderer.act(() => instance.unmount()); + + isInFlightFn.mockClear(); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + expect(isInFlightFn).toBeCalledTimes(0); + expect(console.error).toBeCalledTimes(0); + }); + + it('does not dispose previous in-flight mutaiton ', () => { + const onCompleted = jest.fn(); + render(environment, CommentCreateMutation); + commit({ variables, onCompleted }); + ReactTestRenderer.act(() => instance.unmount()); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + expect(onCompleted).toBeCalledTimes(1); + expect(onCompleted).toBeCalledWith({ + commentCreate: { + feedbackCommentEdge: { + cursor: '', + node: { + id: '', + body: { + text: '', + }, + }, + }, + }, + }); + }); +}); + +describe('optimistic response', () => { + it('calls onCompleted when mutation responses contains server errors', () => { + const onError = jest.fn(); + const onCompleted = jest.fn(); + + render(environment, CommentCreateMutation); + renderSpy.mockClear(); + commit({ variables, onError, onCompleted, optimisticResponse }); + + expectFragmentResult(optimisticResponse, null); + const operation = environment.executeMutation.mock.calls[0][0].operation; + + isInFlightFn.mockClear(); + renderSpy.mockClear(); + ReactTestRenderer.act(() => + environment.mock.resolve(operation, { + data: data.data as PayloadData, + errors: [ + { + message: '', + }, + { + message: '', + }, + ] as Array, + }), + ); + expect(onError).toBeCalledTimes(1); // changed + expect(onCompleted).toBeCalledTimes(0); // changed + const errors = [ + { + message: '', + }, + { + message: '', + }, + ]; + expect(onError).toBeCalledWith(errors); + expect(isInFlightFn).toBeCalledWith(false); + expectFragmentResult(null, errors); + }); + + it('calls onComplete when mutation successfully resolved', () => { + const onError = jest.fn(); + const onCompleted = jest.fn(); + render(environment, CommentCreateMutation); + + renderSpy.mockClear(); + commit({ variables, onError, onCompleted, optimisticResponse }); + + expectFragmentResult(optimisticResponse, null); + + isInFlightFn.mockClear(); + const operation = environment.executeMutation.mock.calls[0][0].operation; + ReactTestRenderer.act(() => environment.mock.resolve(operation, data)); + const result = { + commentCreate: { + feedbackCommentEdge: { + cursor: '', + node: { + id: '', + body: { + text: '', + }, + }, + }, + }, + }; + expect(onError).toBeCalledTimes(0); + expect(onCompleted).toBeCalledTimes(1); + expect(onCompleted).toBeCalledWith(result); + expect(isInFlightFn).toBeCalledWith(false); + expectFragmentResult(result, null); + }); +}); From 0ae7a444e12108d8481017bc3c65f9e2fd0a0bcb Mon Sep 17 00:00:00 2001 From: morrys Date: Fri, 12 Feb 2021 10:33:44 +0100 Subject: [PATCH 2/3] fix optimisticResponse in useMutation --- src/useMutation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/useMutation.ts b/src/useMutation.ts index c2f8b51e..5a98305e 100644 --- a/src/useMutation.ts +++ b/src/useMutation.ts @@ -71,7 +71,7 @@ export function useMutation( setState({ loading: true, - data: optimisticResponse, + data: mergedConfig.optimisticResponse, error: null, }); From fb5be42b43dfa94c145de31ea689b5a3ce0a36a3 Mon Sep 17 00:00:00 2001 From: morrys Date: Fri, 12 Feb 2021 11:08:47 +0100 Subject: [PATCH 3/3] avoid setState on unmount --- __tests__/useMutation-test.tsx | 2 +- src/useMutation.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/__tests__/useMutation-test.tsx b/__tests__/useMutation-test.tsx index 6da6ee6e..b421ad28 100644 --- a/__tests__/useMutation-test.tsx +++ b/__tests__/useMutation-test.tsx @@ -320,7 +320,7 @@ describe('unmount', () => { it('does not setState on complete after unmount', () => { render(environment, CommentCreateMutation); - commit({ variables }); + ReactTestRenderer.act(() => commit({ variables })); ReactTestRenderer.act(() => instance.unmount()); isInFlightFn.mockClear(); diff --git a/src/useMutation.ts b/src/useMutation.ts index 5a98305e..54e40f50 100644 --- a/src/useMutation.ts +++ b/src/useMutation.ts @@ -69,11 +69,13 @@ export function useMutation( invariant(mergedConfig.variables, 'you must specify variables'); - setState({ - loading: true, - data: mergedConfig.optimisticResponse, - error: null, - }); + if (isMounted()) { + setState({ + loading: true, + data: mergedConfig.optimisticResponse, + error: null, + }); + } return new Promise((resolve, reject) => { function handleError(error: any): void {