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 spec compliant default Accept header #618

Merged
merged 1 commit into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const ACCEPT_HEADER = `Accept`
export const CONTENT_TYPE_HEADER = `Content-Type`
export const CONTENT_TYPE_JSON = `application/json`
export const CONTENT_TYPE_GQL = `application/graphql-response+json`
32 changes: 16 additions & 16 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ACCEPT_HEADER, CONTENT_TYPE_GQL, CONTENT_TYPE_HEADER, CONTENT_TYPE_JSON } from './constants.js'
import { defaultJsonSerializer } from './defaultJsonSerializer.js'
import { HeadersInstanceToPlainObject, uppercase } from './helpers.js'
import {
Expand Down Expand Up @@ -133,14 +134,18 @@ const createHttpMethodFetcher =
async <V extends Variables>(params: RequestVerbParams<V>) => {
const { url, query, variables, operationName, fetch, fetchOptions, middleware } = params

const headers = new Headers(params.headers as HeadersInit)
const headers = new Headers(params.headers)
let queryParams = ``
let body = undefined

if (!headers.has(ACCEPT_HEADER)) {
headers.set(ACCEPT_HEADER, [CONTENT_TYPE_GQL, CONTENT_TYPE_JSON].join(`, `))
}

if (method === `POST`) {
body = createRequestBody(query, variables, operationName, fetchOptions.jsonSerializer)
if (typeof body === `string` && !headers.has(`Content-Type`)) {
headers.set(`Content-Type`, `application/json`)
if (typeof body === `string` && !headers.has(CONTENT_TYPE_HEADER)) {
headers.set(CONTENT_TYPE_HEADER, CONTENT_TYPE_JSON)
}
} else {
// @ts-expect-error todo needs ADT for TS to understand the different states
Expand Down Expand Up @@ -619,26 +624,21 @@ const getResult = async (
| { data: undefined; errors: object }
| { data: undefined; errors: object[] }
> => {
let contentType: string | undefined

response.headers.forEach((value, key) => {
if (key.toLowerCase() === `content-type`) {
contentType = value
}
})
const contentType = response.headers.get(CONTENT_TYPE_HEADER)

if (
contentType &&
(contentType.toLowerCase().startsWith(`application/json`) ||
contentType.toLowerCase().startsWith(`application/graphql+json`) ||
contentType.toLowerCase().startsWith(`application/graphql-response+json`))
) {
if (contentType && isJsonContentType(contentType)) {
return jsonSerializer.parse(await response.text()) as any
} else {
return response.text() as any
}
}

const isJsonContentType = (contentType: string) => {
const contentTypeLower = contentType.toLowerCase()

return contentTypeLower.includes(CONTENT_TYPE_GQL) || contentTypeLower.includes(CONTENT_TYPE_JSON)
}

const callOrIdentity = <T>(value: MaybeLazy<T>) => {
return typeof value === `function` ? (value as () => T)() : value
}
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/document-node.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ exports[`accepts graphql DocumentNode as alternative to raw string 1`] = `
}",
},
"headers": {
"accept": "*/*",
"accept": "application/graphql-response+json, application/json",
"accept-encoding": "gzip, deflate",
"accept-language": "*",
"connection": "keep-alive",
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/gql.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ exports[`gql > passthrough allowing benefits of tooling for gql template tag 1`]
}",
},
"headers": {
"accept": "*/*",
"accept": "application/graphql-response+json, application/json",
"accept-encoding": "gzip, deflate",
"accept-language": "*",
"connection": "keep-alive",
Expand Down
123 changes: 38 additions & 85 deletions tests/general.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { GraphQLClient, rawRequest, request } from '../src/index.js'
import type { RequestConfig } from '../src/types.js'
import { setupMockServer } from './__helpers.js'
import { gql } from 'graphql-tag'
import type { Mock } from 'vitest'
Expand Down Expand Up @@ -62,65 +61,6 @@ test(`minimal raw query with response headers`, async () => {
expect(headers.get(`X-Custom-Header`)).toEqual(reqHeaders![`X-Custom-Header`])
})

test(`minimal raw query with response headers and new graphql content type`, async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test is no longer relevant as this mime-type is no longer spec compliant.

const { headers: _, body } = ctx.res({
headers: {
'Content-Type': `application/graphql+json`,
},
body: {
data: {
me: {
id: `some-id`,
},
},
extensions: {
version: `1`,
},
},
}).spec

const { headers: __, ...result } = await rawRequest(ctx.url, `{ me { id } }`)

expect(result).toEqual({ ...body, status: 200 })
})

test(`minimal raw query with response headers and application/graphql-response+json response type`, async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We no longer need this test as this header is now sent by default. Also this wrongly set the Content-Type header, whereas the content-type should always be application/json.

const { headers: _, body } = ctx.res({
headers: {
'Content-Type': `application/graphql-response+json`,
},
body: {
data: {
me: {
id: `some-id`,
},
},
extensions: {
version: `1`,
},
},
}).spec

const { headers: __, ...result } = await rawRequest(ctx.url, `{ me { id } }`)

expect(result).toEqual({ ...body, status: 200 })
})

test(`content-type with charset`, async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test was not doing anything, the relevant code was already commented out.

const { data } = ctx.res({
// headers: { 'Content-Type': 'application/json; charset=utf-8' },
body: {
data: {
me: {
id: `some-id`,
},
},
},
}).spec.body!

expect(await request(ctx.url, `{ me { id } }`)).toEqual(data)
})

test(`basic error`, async () => {
ctx.res({
body: {
Expand Down Expand Up @@ -336,31 +276,6 @@ test.skip(`extra fetch options`, async () => {
`)
})

test(`case-insensitive content-type header for custom fetch`, async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was testing internal behavior of Headers, so this should no longer be needed.

const testData = { data: { test: `test` } }
const testResponseHeaders = new Map()
testResponseHeaders.set(`ConTENT-type`, `apPliCatiON/JSON`)

const options: RequestConfig = {
// @ts-expect-error testing
fetch: (url) =>
Promise.resolve({
headers: testResponseHeaders,
data: testData,
json: () => testData,
text: () => JSON.stringify(testData),
ok: true,
status: 200,
url,
}),
}

const client = new GraphQLClient(ctx.url, options)
const result = await client.request(`{ test }`)

expect(result).toEqual(testData.data)
})

describe(`operationName parsing`, () => {
it(`should work for gql documents`, async () => {
const mock = ctx.res({ body: { data: { foo: 1 } } })
Expand Down Expand Up @@ -405,3 +320,41 @@ test(`should not throw error when errors property is an empty array (occurred wh

expect(res).toEqual(expect.objectContaining({ test: `test` }))
})

it(`adds the default headers to the request`, async () => {
const mock = ctx.res({ body: { data: {} } })
await request(
ctx.url,
gql`
query myGqlOperation {
users
}
`,
)

const headers = mock.requests[0]?.headers
expect(headers?.[`accept`]).toEqual(`application/graphql-response+json, application/json`)
expect(headers?.[`content-type`]).toEqual(`application/json`)
})

it(`allows overriding the default headers for the request`, async () => {
const mock = ctx.res({ body: { data: {} } })
const query = gql`
query myGqlOperation {
users
}
`

await request({
url: ctx.url,
document: query,
requestHeaders: {
accept: `text/plain`,
'content-type': `text/plain`,
},
})

const headers = mock.requests[0]?.headers
expect(headers?.[`accept`]).toEqual(`text/plain`)
expect(headers?.[`content-type`]).toEqual(`text/plain`)
})
18 changes: 0 additions & 18 deletions tests/headers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,24 +107,6 @@ describe(`using class`, () => {
expect(mock.requests[0]?.headers[`x-foo`]).toEqual(`new`)
})
})

describe(`allows content-type header to be overwritten`, () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test was moved into the main suite.

test(`with request method`, async () => {
const headers = new Headers({ 'content-type': `text/plain` })
const client = new GraphQLClient(ctx.url, { headers })
const mock = ctx.res()
await client.request(`{ me { id } }`)
expect(mock.requests[0]?.headers[`content-type`]).toEqual(`text/plain`)
})

test(`with rawRequest method`, async () => {
const headers = new Headers({ 'content-type': `text/plain` })
const client = new GraphQLClient(ctx.url, { headers })
const mock = ctx.res()
await client.rawRequest(`{ me { id } }`)
expect(mock.requests[0]?.headers[`content-type`]).toEqual(`text/plain`)
})
})
})
})

Expand Down