Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Improve mismatched query error message #2883

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moretti
Copy link

@moretti moretti commented Mar 18, 2019

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests
  • If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

This PR improves the error message generated by MockLink / MockedProvider when a query or mutation does not match with any of the mocked requests.


For example, the following mock:

{
  request: {
    query: gql`
      query User {
        user {
          name
        }
      }
    `,
  },
  result: {
    data: {
      user: {
        __typename: 'User',
        name: 'Foo',
      },
    },
  },
}

when called with a different query:

query User {
  user {
    id # <- extra field
    name
  }
}

will print the following error:

Screenshot 2019-03-18 at 21 50 55

so that it's easy to understand that we passed an extra field (id in this case)

@apollo-cla
Copy link

@moretti: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@moretti moretti force-pushed the improve-mismatched-query-errors branch from 595c2ce to 158b428 Compare March 18, 2019 14:45
@hwillson
Copy link
Member

Thanks for this PR @moretti. We're a bit sensitive to bundle sizes, so adding new external dependencies to improve error messages isn't something that we're keen on doing. Any ideas around making these error messages better, while at the same time avoiding new deps and keeping an eye on code size?

@moretti
Copy link
Author

moretti commented Mar 21, 2019

@hwillson thank you for your feedback.


I switched from lodash.isequal to lodash because single utility functions can be imported with lodash/func:

// the following imports *should* be equivalent, in terms of bundle size:
import func from 'lodash/func';
import func from 'lodash.func';

To be honest, the only reason why I did that is because I got a TypeScript error when using Object.values here:

const queryDiffs = (<string[]> []).concat(
...values(this.mockedResponsesByKey).map(mockedResponses =>
mockedResponses.map(mockedResponse =>
diffRequest(mockedResponse.request, operation, this.addTypename),
),
),
);

Property 'values' does not exist on type 'ObjectConstructor'

possibly because the TypeScript compiler is configured to use es5 rather than es2015+


I'll see if I can find a smaller alternative to jest-diff, I didn't think that the bundle size would be a problem, because MockedLink and MockedProvider are only used when writing tests, so bundle size should not increase for production builds.

@SimenB
Copy link

SimenB commented Aug 9, 2019

Now that @apollo/react-testing is its own package, bundle size shouldn't really be an issue anymore, should it? And of course I'm biased, but jest-diff is pretty good 😀

(I realise this has been added to the 3.1 milestone, so this might not be a new point 🙂)

@ARMATAV
Copy link

ARMATAV commented Sep 27, 2019

@hwillson Bundle size seems good now - what needs fixing for this to make it to the master branch?

Screen Shot 2019-09-26 at 6 54 16 PM

@moretti
Copy link
Author

moretti commented Sep 27, 2019

I think that the bundle size was never a problem, even in v2 MockedProvider and MockLink
lived in a separate entry point:

import { MockedProvider, MockLink } from 'react-apollo/test-utils';

so they won't affect the size of the production bundle (the same concept applies, for example, to react-dom/test-utils).

I can fix the conflicts and reapply the same changes to @apollo/react-testing.

@ARMATAV
Copy link

ARMATAV commented Sep 29, 2019

That would be awesome @moretti - this legit would've saved me hours of on-the-Caltrain debugging.

@ARMATAV
Copy link

ARMATAV commented Sep 29, 2019

@benjamn @hwillson are there any blockers besides the conflicting files that should be taken care of?

@apieceofbart
Copy link

What is the status of this, do you need some help? This could help a lot of people

@priley86
Copy link

priley86 commented Feb 17, 2020

This fix would be a huge addition for anyone using Apollo Client 2, I certainly hope it is in some way ported into Apollo Client 3. Right now, I am attempting to port this into @apollo/react-testing for my setup w/ Apollo Client 2 and it looks fairly close to what's been written in mockLink.ts here:

https://github.com/apollographql/react-apollo/blob/v2.5.8/packages/testing/src/mocks/mockLink.ts

I am doing a fair amount of mocking and this is a huge huge need so thanks a lot for considering this. It goes all long ways for writing more testable apps w/ Apollo Client and saving hours of headache.

@dacevedo12
Copy link

Any news? This would be really useful

@EoinF
Copy link

EoinF commented Mar 26, 2020

This would be really useful for me too. It's going to be so hard to debug failing tests without this :/

@hwillson hwillson removed their request for review April 8, 2020 11:55
@hamishtaplin
Copy link

What's the status on this? Would solve by far the biggest pain working with MockedProvider

@priley86
Copy link

for anyone out there needing a temp solution for Apollo 2.x, I have forked @apollo/react-testing and provided this as an NPM here:
https://www.npmjs.com/package/@houselanister/react-testing

"@houselanister/react-testing": "3.1.6",

Commit:
https://github.com/priley86/react-apollo/commits/mismatch-query

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants