react-apollo icon indicating copy to clipboard operation
react-apollo copied to clipboard

Improve mismatched query error message

Open moretti opened this issue 6 years ago • 14 comments
trafficstars

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
  • [x] 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)

moretti avatar Mar 18 '19 14:03 moretti

@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/

apollo-cla avatar Mar 18 '19 14:03 apollo-cla

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?

hwillson avatar Mar 21 '19 13:03 hwillson

@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:

https://github.com/apollographql/react-apollo/blob/158b428356a1e66b1685cd43882b839ba16a8c5b/src/test-links.ts#L81-L87

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.

moretti avatar Mar 21 '19 13:03 moretti

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 🙂)

SimenB avatar Aug 09 '19 13:08 SimenB

@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

ARMATAV avatar Sep 27 '19 01:09 ARMATAV

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.

moretti avatar Sep 27 '19 08:09 moretti

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

ARMATAV avatar Sep 29 '19 22:09 ARMATAV

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

ARMATAV avatar Sep 29 '19 22:09 ARMATAV

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

apieceofbart avatar Jan 17 '20 15:01 apieceofbart

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.

priley86 avatar Feb 17 '20 18:02 priley86

Any news? This would be really useful

dacevedo12 avatar Feb 20 '20 22:02 dacevedo12

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

EoinF avatar Mar 26 '20 11:03 EoinF

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

hamishtaplin avatar Apr 15 '20 09:04 hamishtaplin

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

priley86 avatar Jun 16 '20 17:06 priley86