Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Look through definitions to find an operation type #42

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

Conversation

amcvitty
Copy link

@amcvitty amcvitty commented Feb 14, 2019

Why
It's weird that often the link logs with undefined type

What:

For documents with queries and fragment definitions, often I was noticing that the operation type was reported as undefined.

image

The root cause is that we were looking at the first definition in the query, which in our app is often a Fragment Definition instead of an Operation Definition and so doesn't have an operationType (query/mutation etc).

image

So instead since most documents have a single operation, I'm trying to find and report its operationType instead of looking only at the first definition.

How:
This change simply looks through to find one that isn't undefined. It's still picking one from an array, but I think this is generally going to give better results than the previous heuristic of taking the first.

Checklist:

  • Documentation N/A
  • Tests N/A
  • Ready to be merged
  • Added myself to contributors table

@ghost
Copy link

ghost commented Feb 14, 2019

There were the following issues with this Pull Request

  • Commit: 08100b5
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@amcvitty amcvitty force-pushed the no-undefined-operation-types branch 2 times, most recently from f317dca to fb64a8f Compare February 14, 2019 07:42
Previously we were just reporting the first, which for large queries was
often a fragment definition, which doesn't have an operation. In most
use cases, there will be exactly one query or mutation. In the case
where there are multiple, we'll report one of them (no worse than
before!). Also - correct spelling of "elapsed"
@amcvitty amcvitty force-pushed the no-undefined-operation-types branch from fb64a8f to 16aff60 Compare February 14, 2019 12:15
@amcvitty
Copy link
Author

Hi - I just checked back on this because I'm still a bit frustrated by the undefined showing up in my logs all the time.

Any feedback on this PR? Can a maintainer help me understand what more I would need to do to get this merged?

Please note that I've already corrected the issues identified by commitlint.

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

Successfully merging this pull request may close these issues.

1 participant