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

feat: add operationName to payload if defined in gql #280

Merged
merged 1 commit into from
Jul 26, 2021

Conversation

mydea
Copy link
Contributor

@mydea mydea commented Jul 26, 2021

We use the operation name for tracking purposes on our API. However, as already discussed here: #64, the operationName is currently not send.

In contrast to issue #64, which is about allowing to define an operationName when sending multiple queries at once, this is about sending the operationName field in the payload sent to the API for "regular" queries/mutations like this:

query allUsers {
  users
}

Where I want the server to use allUsers for tracking purposes. When we send the operationName field in our payload, like e.g. Apollo or some other GraphQL clients do, it can easily be used by GraphQL server implementations. This should be a completely backwards compatible change.

I tried to build this in a completely backwards-compatible way by extracting the actual fetch call from rawRequest into a private makeRequest function, which is called by both rawRequest and request. This way, we can pass in the (optional) operationName extracted from the query, if possible, and use that, without changing the signature of any public method.
This will basically only do anything if sending a single operation with an operation name present. Otherwise, nothing will change. If not passing a DocumentNode as query, but just a string, it will also do nothing.

I also moved the header-normalization out from the post/get methods to the request methods, to keep the get/post methods as minimal as possible.

requestHeaders?: Dom.RequestInit['headers'],
) => {
const body = createRequestBody(query, variables)
const post = async <V = Variables>({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the signature like this is a breaking change.

Add an overload instead for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm but isn't this method private only? It is not exported, so I thought it would be easier to change it that way.
I can also add it as optional last parameter, though, if you prefer!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh if its not part of the API then disregard this comment.

Copy link
Member

@jasonkuhrt jasonkuhrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jasonkuhrt jasonkuhrt changed the title feat: Add operationName to payload if defined in gql feat: add operationName to payload if defined in gql Jul 26, 2021
@jasonkuhrt jasonkuhrt merged commit 20f18d3 into graffle-js:master Jul 26, 2021
@mydea mydea deleted the fn/auto-capture-operation-name branch July 27, 2021 06:56
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.

2 participants