Don't mutate options object when calculating variables from props (#1968)

* Code formatting; Test; Chanelog update

Author: ksmth

Changelog.md

@@ -10,6 +10,11 @@
   instance in `client`, and the received subscription data in
   `subscriptionData`. <br/>
   [@jedwards1211](https://github.com/jedwards1211) in [#1966](https://github.com/apollographql/react-apollo/pull/1966)
+- The `graphql` `options` object is no longer mutated, when calculating
+  variables from props. This now prevents an issue where components created
+  with `graphql` were not having their query variables updated properly, when
+  props changed.
+  [@ksmth](https://github.com/ksmth) in [#1968](https://github.com/apollographql/react-apollo/pull/1968)
 
 ## 2.1.11 (August 9, 2018)
 

src/query-hoc.tsx

@@ -33,10 +33,14 @@ export function query<
   } = operationOptions;
 
   let mapPropsToOptions = options as (props: any) => QueryOpts;
-  if (typeof mapPropsToOptions !== 'function') mapPropsToOptions = () => options as QueryOpts;
+  if (typeof mapPropsToOptions !== 'function') {
+    mapPropsToOptions = () => options as QueryOpts;
+  }
 
   let mapPropsToSkip = skip as (props: any) => boolean;
-  if (typeof mapPropsToSkip !== 'function') mapPropsToSkip = () => skip as any;
+  if (typeof mapPropsToSkip !== 'function') {
+    mapPropsToSkip = () => skip as any;
+  }
 
   // allow for advanced referential equality checks
   let lastResultProps: TChildProps | void;
@@ -51,7 +55,7 @@ export function query<
       render() {
         let props = this.props;
         const shouldSkip = mapPropsToSkip(props);
-        const opts = shouldSkip ? Object.create(null) : mapPropsToOptions(props);
+        const opts = shouldSkip ? Object.create(null) : { ...mapPropsToOptions(props) };
 
         if (!shouldSkip && !opts.variables && operation.variables.length > 0) {
           opts.variables = calculateVariablesFromProps(

test/client/graphql/queries/index.test.tsx

@@ -7,6 +7,7 @@ import { InMemoryCache as Cache } from 'apollo-cache-inmemory';
 import { ApolloLink } from 'apollo-link';
 import { mockSingleLink } from '../../../../src/test-utils';
 import { ApolloProvider, graphql, DataProps, ChildProps } from '../../../../src';
+import { mount } from 'enzyme';
 
 import stripSymbols from '../../../test-utils/stripSymbols';
 import catchAsyncError from '../../../test-utils/catchAsyncError';
@@ -112,6 +113,72 @@ describe('queries', () => {
     );
   });
 
+  it('should update query variables when props change', () => {
+    const query: DocumentNode = gql`
+      query people($someId: ID) {
+        allPeople(someId: $someId) {
+          people {
+            name
+          }
+        }
+      }
+    `;
+
+    const link = mockSingleLink(
+      {
+        request: { query, variables: { someId: 1 } },
+        result: { data: { allPeople: { people: [{ name: 'Luke Skywalker' }] } } },
+      },
+      {
+        request: { query, variables: { someId: 2 } },
+        result: { data: { allPeople: { people: [{ name: 'Darth Vader' }] } } },
+      },
+    );
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    interface Data {
+      allPeople: {
+        people: {
+          name: string;
+        };
+      };
+    }
+
+    interface Variables {
+      someId: number;
+    }
+
+    const options = {
+      options: {},
+    };
+
+    let count = 0;
+    const ContainerWithData = graphql<Variables, Data, Variables>(query, options)(
+      ({ data }: ChildProps<Variables, Data, Variables>) => {
+        expect(data).toBeTruthy();
+        if (count === 0) {
+          expect(data!.variables.someId).toEqual(1);
+        } else if (count === 1) {
+          expect(data!.variables.someId).toEqual(2);
+        }
+        count += 1;
+        return null;
+      },
+    );
+
+    const wrapper = mount(
+      <ApolloProvider client={client}>
+        <ContainerWithData someId={1} />
+      </ApolloProvider>,
+    );
+    wrapper.setProps({
+      children: React.cloneElement(wrapper.props().children, { someId: 2 }),
+    });
+  });
+
   it("shouldn't warn about fragments", () => {
     const oldWarn = console.warn;
     const warnings: any[] = [];