diff --git a/packages/react-graphql/CHANGELOG.md b/packages/react-graphql/CHANGELOG.md index ca426eb709..bc026eb490 100644 --- a/packages/react-graphql/CHANGELOG.md +++ b/packages/react-graphql/CHANGELOG.md @@ -7,6 +7,13 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [3.3.6] - 2019-05-31 + +### Fixed + +- Queries are no longer waited on during server render when `skip` is `true` ([#726](https://github.com/Shopify/quilt/pull/726)) +- The result of calling `useQuery` is now referentially stable when variables stay deep-equal between calls (previously, using a different object with the same values would change the result) ([#726](https://github.com/Shopify/quilt/pull/726)). + ## [3.3.5] - 2019-05-29 ### Fixed diff --git a/packages/react-graphql/src/hooks/graphql-document.ts b/packages/react-graphql/src/hooks/graphql-document.ts index 3a29f58383..bebd9d8f3c 100644 --- a/packages/react-graphql/src/hooks/graphql-document.ts +++ b/packages/react-graphql/src/hooks/graphql-document.ts @@ -2,6 +2,7 @@ import {useState, useEffect, useCallback} from 'react'; import {OperationVariables} from 'apollo-client'; import {DocumentNode} from 'graphql-typed'; import {useMountedRef} from '@shopify/react-hooks'; +import {useAsyncAsset} from '@shopify/react-async'; import {AsyncQueryComponentType} from '../types'; @@ -13,7 +14,7 @@ export default function useGraphQLDocument< documentOrComponent: | DocumentNode | AsyncQueryComponentType, -): [DocumentNode | null, string | undefined] { +): DocumentNode | null { const [document, setDocument] = useState | null>( () => { if (!query) { @@ -60,8 +66,7 @@ export default function useQuery< query, // eslint-disable-next-line react-hooks/exhaustive-deps context && JSON.stringify(context), - // eslint-disable-next-line react-hooks/exhaustive-deps - variables && JSON.stringify(variables), + serializedVariables, fetchPolicy, errorPolicy, pollInterval, @@ -90,33 +95,9 @@ export default function useQuery< }); const defaultResult = useMemo>( - () => ({ - data: undefined, - error: undefined, - networkStatus: undefined, - loading: false, - variables: queryObservable ? queryObservable.variables : variables, - refetch: queryObservable - ? queryObservable.refetch.bind(queryObservable) - : (noop as any), - fetchMore: queryObservable - ? queryObservable.fetchMore.bind(queryObservable) - : (noop as any), - updateQuery: queryObservable - ? queryObservable.updateQuery.bind(queryObservable) - : (noop as any), - startPolling: queryObservable - ? queryObservable.startPolling.bind(queryObservable) - : (noop as any), - stopPolling: queryObservable - ? queryObservable.stopPolling.bind(queryObservable) - : (noop as any), - subscribeToMore: queryObservable - ? queryObservable.subscribeToMore.bind(queryObservable) - : (noop as any), - client, - }), - [queryObservable, client, variables], + () => createDefaultResult(client, variables, queryObservable), + // eslint-disable-next-line react-hooks/exhaustive-deps + [queryObservable, client, serializedVariables], ); const [responseId, setResponseId] = useState(0); @@ -145,6 +126,7 @@ export default function useQuery< const previousData = useRef< QueryHookResult['data'] | undefined >(undefined); + const currentResult = useMemo>( () => { // must of the logic below are lifted from @@ -202,4 +184,37 @@ export default function useQuery< return currentResult; } +function createDefaultResult( + client: ApolloClient, + variables: any, + queryObservable?: ObservableQuery, +) { + return { + data: undefined, + error: undefined, + networkStatus: undefined, + loading: false, + variables: queryObservable ? queryObservable.variables : variables, + refetch: queryObservable + ? queryObservable.refetch.bind(queryObservable) + : noop, + fetchMore: queryObservable + ? queryObservable.fetchMore.bind(queryObservable) + : noop, + updateQuery: queryObservable + ? queryObservable.updateQuery.bind(queryObservable) + : noop, + startPolling: queryObservable + ? queryObservable.startPolling.bind(queryObservable) + : noop, + stopPolling: queryObservable + ? queryObservable.stopPolling.bind(queryObservable) + : noop, + subscribeToMore: queryObservable + ? queryObservable.subscribeToMore.bind(queryObservable) + : noop, + client, + }; +} + function noop() {} diff --git a/packages/react-graphql/src/hooks/tests/mutation.test.tsx b/packages/react-graphql/src/hooks/tests/mutation.test.tsx index 0d02ec4426..54c5ae89e8 100644 --- a/packages/react-graphql/src/hooks/tests/mutation.test.tsx +++ b/packages/react-graphql/src/hooks/tests/mutation.test.tsx @@ -3,11 +3,7 @@ import gql from 'graphql-tag'; import {createGraphQLFactory} from '@shopify/graphql-testing'; import useMutation from '../mutation'; -import { - mountWithGraphQL, - prepareAsyncReactTasks, - teardownAsyncReactTasks, -} from './utilities'; +import {mountWithGraphQL} from './utilities'; const updatePetMutation = gql` mutation UpdatePetMutation($name: String!) { @@ -41,14 +37,6 @@ const mockMutationData = { }; describe('useMutation', () => { - beforeEach(() => { - prepareAsyncReactTasks(); - }); - - afterEach(() => { - teardownAsyncReactTasks(); - }); - it('returns result that contains the loaded data once the mutation finished running', async () => { const renderPropSpy = jest.fn(() => null); const graphQL = createGraphQL({UpdatePetMutation: mockMutationData}); diff --git a/packages/react-graphql/src/hooks/tests/query.test.tsx b/packages/react-graphql/src/hooks/tests/query.test.tsx index 1ab8fbe14d..1e6918e8a7 100644 --- a/packages/react-graphql/src/hooks/tests/query.test.tsx +++ b/packages/react-graphql/src/hooks/tests/query.test.tsx @@ -7,12 +7,7 @@ import {createGraphQLFactory} from '@shopify/graphql-testing'; import {createAsyncQueryComponent} from '../../async'; import useQuery from '../query'; -import { - mountWithGraphQL, - prepareAsyncReactTasks, - teardownAsyncReactTasks, - createResolvablePromise, -} from './utilities'; +import {mountWithGraphQL, createResolvablePromise} from './utilities'; const petQuery = gql` query PetQuery { @@ -33,14 +28,6 @@ const mockData = { }; describe('useQuery', () => { - beforeEach(() => { - prepareAsyncReactTasks(); - }); - - afterEach(() => { - teardownAsyncReactTasks(); - }); - describe('document', () => { it('returns loading=true and networkStatus=loading during the loading of query', async () => { function MockQuery({children}) { @@ -89,6 +76,43 @@ describe('useQuery', () => { ); }); + it('keeps the same data when the variables stay deep-equal', async () => { + function MockQuery({ + children, + variables, + }: { + children: ( + result: ReturnType, + ) => React.ReactElement | null; + variables?: object; + }) { + const results = useQuery(petQuery, {variables}); + return children(results); + } + + const graphQL = createGraphQL({PetQuery: mockData}); + const renderPropSpy = jest.fn(() => null); + const variables = {foo: 'bar'}; + + const mockQuery = await mountWithGraphQL( + {renderPropSpy}, + { + graphQL, + }, + ); + + mockQuery.setProps({variables: {...variables}}); + + expect(graphQL.operations.all()).toHaveLength(1); + + // Once for initial render while loading, once for when the data loaded, and a final time + // when we update the props and re-render the component. + expect(renderPropSpy).toHaveBeenCalledTimes(3); + + const [, firstLoadedCall, secondLoadedCall] = renderPropSpy.mock.calls; + expect(firstLoadedCall[0]).toBe(secondLoadedCall[0]); + }); + it('watchQuery is not called when skip is true', async () => { const mockClient = createMockApolloClient(); const watchQuerySpy = jest.fn(); diff --git a/packages/react-graphql/src/hooks/tests/utilities.tsx b/packages/react-graphql/src/hooks/tests/utilities.tsx index 4137b97172..d7fd9f27a7 100644 --- a/packages/react-graphql/src/hooks/tests/utilities.tsx +++ b/packages/react-graphql/src/hooks/tests/utilities.tsx @@ -2,7 +2,6 @@ import * as React from 'react'; import {createGraphQLFactory, GraphQL} from '@shopify/graphql-testing'; import {createMount} from '@shopify/react-testing'; -import {promise} from '@shopify/jest-dom-mocks'; import {ApolloProvider} from '../../ApolloProvider'; @@ -37,28 +36,6 @@ export const mountWithGraphQL = createMount({ }, }); -export function prepareAsyncReactTasks() { - if (!promise.isMocked()) { - promise.mock(); - } -} - -export function teardownAsyncReactTasks() { - if (promise.isMocked()) { - promise.restore(); - } -} - -export function runPendingAsyncReactTasks() { - if (!promise.isMocked()) { - throw new Error( - 'You attempted to resolve pending async React tasks, but have not yet prepared to do so. Run `prepareAsyncReactTasks()` from "tests/modern" in a `beforeEach` block and try again.', - ); - } - - promise.runPending(); -} - export function createResolvablePromise(value: T) { let resolver!: () => Promise; let rejecter!: () => void; diff --git a/packages/react-testing/CHANGELOG.md b/packages/react-testing/CHANGELOG.md index f58d3930c5..f84ff451fb 100644 --- a/packages/react-testing/CHANGELOG.md +++ b/packages/react-testing/CHANGELOG.md @@ -7,6 +7,16 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [1.5.4] - 2019-05-31 + +### Fixed + +- When using a custom mount with `createMount`, calling `setProps` on the resulting elements will now properly set props on the JSX that was mounted, not the element returned from the `createMount` `render` option ([#726](https://github.com/Shopify/quilt/pull/726)). + + > **Note**: In order to support the above, a small change was made to the `Root` class’s constructor. If you were calling this directly (which is discouraged), you will need to use the new `resolveRoot` option instead of the existing second argument. Additionally, if you were manually passing through additional props in a component you used to wrap elements in `createMount.render`, you can now remove this workaround. + +## [1.5.3] - 2019-05-22 + ### Fixed - Passing unresolved promises within `act()` blocks required additional nesting ([#697](https://github.com/Shopify/quilt/pull/697)) diff --git a/packages/react-testing/src/TestWrapper.tsx b/packages/react-testing/src/TestWrapper.tsx index 70ea008789..3a1bb2a083 100644 --- a/packages/react-testing/src/TestWrapper.tsx +++ b/packages/react-testing/src/TestWrapper.tsx @@ -6,6 +6,7 @@ interface State { interface Props { children: React.ReactElement; + render(element: React.ReactElement): React.ReactElement; } export class TestWrapper extends React.Component< @@ -21,7 +22,7 @@ export class TestWrapper extends React.Component< render() { const {props} = this.state; - const {children} = this.props; - return props ? React.cloneElement(children, props) : children; + const {children, render} = this.props; + return render(props ? React.cloneElement(children, props) : children); } } diff --git a/packages/react-testing/src/mount.ts b/packages/react-testing/src/mount.ts index 963d546d99..cdd03eba8f 100644 --- a/packages/react-testing/src/mount.ts +++ b/packages/react-testing/src/mount.ts @@ -1,6 +1,6 @@ import * as React from 'react'; import {IfAllOptionalKeys} from '@shopify/useful-types'; -import {Root} from './root'; +import {Root, Options as RootOptions} from './root'; import {Element} from './element'; export {Root, Element}; @@ -81,9 +81,9 @@ export class CustomRoot extends Root { constructor( tree: React.ReactElement, public readonly context: Context, - resolve: (element: Element) => Element | null, + options?: RootOptions, ) { - super(tree, resolve); + super(tree, options); } } @@ -105,11 +105,11 @@ export function createMount< options: MountOptions = {} as any, ) { const context = createContext(options); - const rendered = render(element, context, options); - const wrapper = new CustomRoot(rendered, context, root => - root.find(element.type), - ); + const wrapper = new CustomRoot(element, context, { + render: element => render(element, context, options), + resolveRoot: root => root.find(element.type), + }); const afterMountResult = afterMount(wrapper, options); diff --git a/packages/react-testing/src/root.tsx b/packages/react-testing/src/root.tsx index dc03f799e6..51027b3050 100644 --- a/packages/react-testing/src/root.tsx +++ b/packages/react-testing/src/root.tsx @@ -26,6 +26,14 @@ const act = reactAct as (func: () => void | Promise) => Promise; const {findCurrentFiberUsingSlowPath} = require('react-reconciler/reflection'); type ResolveRoot = (element: Element) => Element | null; +type Render = ( + element: React.ReactElement, +) => React.ReactElement; + +export interface Options { + render?: Render; + resolveRoot?: ResolveRoot; +} export const connected = new Set>(); @@ -67,14 +75,20 @@ export class Root { private root: Element | null = null; private acting = false; + private render: Render; + private resolveRoot: ResolveRoot; + private get mounted() { return this.wrapper != null; } constructor( private tree: React.ReactElement, - private resolveRoot: ResolveRoot = defaultResolveRoot, + {render = defaultRender, resolveRoot = defaultResolveRoot}: Options = {}, ) { + this.render = render; + this.resolveRoot = resolveRoot; + this.mount(); } @@ -179,6 +193,7 @@ export class Root { this.act(() => { render( + render={this.render} ref={wrapper => { this.wrapper = wrapper; }} @@ -257,6 +272,10 @@ function defaultResolveRoot(element: Element) { return element.children[0]; } +function defaultRender(element: React.ReactElement) { + return element; +} + function flatten( element: Fiber, root: Root, diff --git a/packages/react-testing/src/tests/mount.test.tsx b/packages/react-testing/src/tests/mount.test.tsx index e7bfede65b..26b6d1f6ad 100644 --- a/packages/react-testing/src/tests/mount.test.tsx +++ b/packages/react-testing/src/tests/mount.test.tsx @@ -1,4 +1,5 @@ import * as React from 'react'; +import {random} from 'faker'; import {Root} from '../root'; import {mount, createMount} from '../mount'; @@ -72,6 +73,29 @@ describe('createMount()', () => { expect(div).not.toContainReactComponent('span', {id: 'ShouldNotBeFound'}); }); + it('can set props on a nested element even if it is wrapped in providers', () => { + function TestComponent({words}: {words: string}) { + return
{words}
; + } + + function Wrapper({children}: {children: React.ReactElement}) { + return children; + } + + const customMount = createMount({ + render: element => {element}, + }); + + const originalWords = random.words(); + const updatedWords = random.words(); + const testComponent = customMount(); + + testComponent.setProps({words: updatedWords}); + + expect(testComponent).not.toContainReactText(originalWords); + expect(testComponent).toContainReactText(updatedWords); + }); + it('calls afterMount with the wrapper and options', () => { const spy = jest.fn(); const options = {foo: 'bar'};