Merge pull request #3146 from marmelab/fix-query-component-update

[RFR] Fix query component does not fetch again when updated
marmelab · Apr 19, 2019 · 5ec9cd6 · 5ec9cd6
2 parents eff280e + 5d621b9
commit 5ec9cd6
Show file tree

Hide file tree

Showing 3 changed files with 169 additions and 37 deletions.
diff --git a/packages/ra-core/src/util/Query.spec.tsx b/packages/ra-core/src/util/Query.spec.tsx
@@ -4,7 +4,7 @@ import {
     cleanup,
     fireEvent,
     // @ts-ignore
-    waitForDomChange
+    waitForDomChange,
 } from 'react-testing-library';
 import expect from 'expect';
 import Query from './Query';
@@ -36,7 +36,11 @@ describe('Query', () => {
                 {({ store }) => {
                     dispatchSpy = jest.spyOn(store, 'dispatch');
                     return (
-                        <Query type="mytype" resource="myresource" payload={myPayload}>
+                        <Query
+                            type="mytype"
+                            resource="myresource"
+                            payload={myPayload}
+                        >
                             {() => <div>Hello</div>}
                         </Query>
                     );
@@ -55,8 +59,16 @@ describe('Query', () => {
         const { getByText } = render(
             <TestContext>
                 {() => (
-                    <Query type="mytype" resource="myresource" payload={myPayload}>
-                        {({ loading }) => <div className={loading ? 'loading' : 'idle'}>Hello</div>}
+                    <Query
+                        type="mytype"
+                        resource="myresource"
+                        payload={myPayload}
+                    >
+                        {({ loading }) => (
+                            <div className={loading ? 'loading' : 'idle'}>
+                                Hello
+                            </div>
+                        )}
                     </Query>
                 )}
             </TestContext>
@@ -66,11 +78,16 @@ describe('Query', () => {
 
     it('should update the data state after a success response', async () => {
         const dataProvider = jest.fn();
-        dataProvider.mockImplementationOnce(() => Promise.resolve({ data: { foo: 'bar' } }));
+        dataProvider.mockImplementationOnce(() =>
+            Promise.resolve({ data: { foo: 'bar' } })
+        );
         const Foo = () => (
             <Query type="mytype" resource="foo">
                 {({ loading, data }) => (
-                    <div data-testid="test" className={loading ? 'loading' : 'idle'}>
+                    <div
+                        data-testid="test"
+                        className={loading ? 'loading' : 'idle'}
+                    >
                         {data ? data.foo : 'no data'}
                     </div>
                 )}
@@ -91,12 +108,17 @@ describe('Query', () => {
 
     it('should return the total prop if available', async () => {
         const dataProvider = jest.fn();
-        dataProvider.mockImplementationOnce(() => Promise.resolve({ data: [{ foo: 'bar' }], total: 42 }));
+        dataProvider.mockImplementationOnce(() =>
+            Promise.resolve({ data: [{ foo: 'bar' }], total: 42 })
+        );
 
         const Foo = () => (
             <Query type="mytype" resource="foo">
                 {({ loading, data, total }) => (
-                    <div data-testid="test" className={loading ? 'loading' : 'idle'}>
+                    <div
+                        data-testid="test"
+                        className={loading ? 'loading' : 'idle'}
+                    >
                         {loading ? 'no data' : total}
                     </div>
                 )}
@@ -120,11 +142,16 @@ describe('Query', () => {
 
     it('should update the error state after an error response', async () => {
         const dataProvider = jest.fn();
-        dataProvider.mockImplementationOnce(() => Promise.reject({ message: 'provider error' }));
+        dataProvider.mockImplementationOnce(() =>
+            Promise.reject({ message: 'provider error' })
+        );
         const Foo = () => (
             <Query type="mytype" resource="foo">
                 {({ loading, error }) => (
-                    <div data-testid="test" className={loading ? 'loading' : 'idle'}>
+                    <div
+                        data-testid="test"
+                        className={loading ? 'loading' : 'idle'}
+                    >
                         {error ? error.message : 'no data'}
                     </div>
                 )}
@@ -142,4 +169,80 @@ describe('Query', () => {
         expect(testElement.textContent).toEqual('provider error');
         expect(testElement.className).toEqual('idle');
     });
+
+    it('should dispatch a new fetch action when updating', () => {
+        let dispatchSpy;
+        const myPayload = {};
+        const { rerender } = render(
+            <TestContext>
+                {({ store }) => {
+                    dispatchSpy = jest.spyOn(store, 'dispatch');
+                    return (
+                        <Query
+                            type="mytype"
+                            resource="myresource"
+                            payload={myPayload}
+                        >
+                            {() => <div>Hello</div>}
+                        </Query>
+                    );
+                }}
+            </TestContext>
+        );
+        const mySecondPayload = { foo: 1 };
+        rerender(
+            <TestContext>
+                {() => (
+                    <Query
+                        type="mytype"
+                        resource="myresource"
+                        payload={mySecondPayload}
+                    >
+                        {() => <div>Hello</div>}
+                    </Query>
+                )}
+            </TestContext>
+        );
+        expect(dispatchSpy.mock.calls.length).toEqual(2);
+        const action = dispatchSpy.mock.calls[1][0];
+        expect(action.type).toEqual('CUSTOM_FETCH');
+        expect(action.payload).toEqual(mySecondPayload);
+        expect(action.meta.fetch).toEqual('mytype');
+        expect(action.meta.resource).toEqual('myresource');
+    });
+
+    it('should not dispatch a new fetch action when updating with the same query props', () => {
+        let dispatchSpy;
+        const myPayload = {};
+        const { rerender } = render(
+            <TestContext>
+                {({ store }) => {
+                    dispatchSpy = jest.spyOn(store, 'dispatch');
+                    return (
+                        <Query
+                            type="mytype"
+                            resource="myresource"
+                            payload={myPayload}
+                        >
+                            {() => <div>Hello</div>}
+                        </Query>
+                    );
+                }}
+            </TestContext>
+        );
+        rerender(
+            <TestContext>
+                {() => (
+                    <Query
+                        type="mytype"
+                        resource="myresource"
+                        payload={myPayload}
+                    >
+                        {() => <div>Hello</div>}
+                    </Query>
+                )}
+            </TestContext>
+        );
+        expect(dispatchSpy.mock.calls.length).toEqual(1);
+    });
 });
diff --git a/packages/ra-core/src/util/Query.tsx b/packages/ra-core/src/util/Query.tsx
@@ -1,7 +1,13 @@
 import { Component, ReactNode } from 'react';
+import { shallowEqual } from 'recompose';
 import withDataProvider from './withDataProvider';
 
-type DataProviderCallback = (type: string, resource: string, payload?: any, options?: any) => Promise<any>;
+type DataProviderCallback = (
+    type: string,
+    resource: string,
+    payload?: any,
+    options?: any
+) => Promise<any>;
 
 interface ChildrenFuncParams {
     data?: any;
@@ -72,27 +78,42 @@ class Query extends Component<Props, State> {
         data: null,
         total: null,
         loading: true,
-        error: null
+        error: null,
     };
 
-    componentDidMount = () => {
+    callDataProvider = () => {
         const { dataProvider, type, resource, payload, options } = this.props;
         dataProvider(type, resource, payload, options)
             .then(({ data, total }) => {
                 this.setState({
                     data,
                     total,
-                    loading: false
+                    loading: false,
                 });
             })
             .catch(error => {
                 this.setState({
                     error,
-                    loading: false
+                    loading: false,
                 });
             });
     };
 
+    componentDidMount = () => {
+        this.callDataProvider();
+    };
+
+    componentDidUpdate = prevProps => {
+        if (
+            prevProps.type !== this.props.type ||
+            prevProps.resource !== this.props.resource ||
+            !shallowEqual(prevProps.payload, this.props.payload) ||
+            !shallowEqual(prevProps.options, this.props.options)
+        ) {
+            this.callDataProvider();
+        }
+    };
+
     render() {
         const { children } = this.props;
         return children(this.state);

diff --git a/packages/ra-core/src/util/TestContext.tsx b/packages/ra-core/src/util/TestContext.tsx
@@ -1,4 +1,4 @@
-import React, { SFC } from 'react';
+import React, { Component } from 'react';
 import { createStore } from 'redux';
 import { Provider } from 'react-redux';
 import { reducer as formReducer } from 'redux-form';
@@ -47,29 +47,37 @@ interface Props {
  *     </TestContext>
  * );
  */
-const TestContext: SFC<Props> = ({
-    store = {},
-    enableReducers = false,
-    children,
-}) => {
-    const storeWithDefault = enableReducers
-        ? createAdminStore({
-              initialState: merge(defaultStore, store),
-              dataProvider: () => Promise.resolve({}),
-              history: createMemoryHistory(),
-          })
-        : createStore(() => merge(defaultStore, store));
+class TestContext extends Component<Props> {
+    storeWithDefault = null;
 
-    const renderChildren = () =>
-        typeof children === 'function'
-            ? children({ store: storeWithDefault })
+    constructor(props) {
+        super(props);
+        const { store = {}, enableReducers = false } = props;
+        this.storeWithDefault = enableReducers
+            ? createAdminStore({
+                  initialState: merge(defaultStore, store),
+                  dataProvider: () => Promise.resolve({}),
+                  history: createMemoryHistory(),
+              })
+            : createStore(() => merge(defaultStore, store));
+    }
+
+    renderChildren = () => {
+        const { children } = this.props;
+        return typeof children === 'function'
+            ? children({ store: this.storeWithDefault })
             : children;
+    };
 
-    return (
-        <Provider store={storeWithDefault}>
-            <TranslationProvider>{renderChildren()}</TranslationProvider>
-        </Provider>
-    );
-};
+    render() {
+        return (
+            <Provider store={this.storeWithDefault}>
+                <TranslationProvider>
+                    {this.renderChildren()}
+                </TranslationProvider>
+            </Provider>
+        );
+    }
+}
 
 export default TestContext;