Skip to content

Commit

Permalink
Fix headers type (extends #9042) (#10471)
Browse files Browse the repository at this point in the history
* fix(types): use correct type for headers

When passing the headers property to `createHttpLink` one might think
that it is ok to pass them as `Headers` interface, but that doesn't work
because there is several places in the code that expect headers to be
passed as an object.

* chore: fix TS errors

* chore: adds changeset

Co-authored-by: Filipe Guerra <alias.mac@gmail.com>
  • Loading branch information
alessbell and alias-mac committed Jan 23, 2023
1 parent 328c58f commit 895ddcb
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/proud-months-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

More robust type definition for `headers` property passed to `createHttpLink`
8 changes: 4 additions & 4 deletions src/link/batch-http/__tests__/batchHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ describe('SharedHttpTest', () => {

execute(link, { query: sampleQuery, variables }).subscribe(
makeCallback(resolve, reject, (result: any) => {
const headers: any = fetchMock.lastCall()![1]!.headers;
const headers: Record<string, string> = fetchMock.lastCall()![1]!.headers as Record<string, string>;
expect(headers.authorization).toBe('1234');
expect(headers['content-type']).toBe('application/json');
expect(headers.accept).toBe('*/*');
Expand All @@ -527,7 +527,7 @@ describe('SharedHttpTest', () => {

execute(link, { query: sampleQuery, variables }).subscribe(
makeCallback(resolve, reject, (result: any) => {
const headers: any = fetchMock.lastCall()![1]!.headers;
const headers: Record<string, string> = fetchMock.lastCall()![1]!.headers as Record<string, string>;
expect(headers.authorization).toBe('1234');
expect(headers['content-type']).toBe('application/json');
expect(headers.accept).toBe('*/*');
Expand All @@ -549,7 +549,7 @@ describe('SharedHttpTest', () => {

execute(link, { query: sampleQuery, variables }).subscribe(
makeCallback(resolve, reject, (result: any) => {
const headers: any = fetchMock.lastCall()![1]!.headers;
const headers: Record<string, string> = fetchMock.lastCall()![1]!.headers as Record<string, string>;
expect(headers.authorization).toBe('1234');
expect(headers['content-type']).toBe('application/json');
expect(headers.accept).toBe('*/*');
Expand All @@ -570,7 +570,7 @@ describe('SharedHttpTest', () => {
context,
}).subscribe(
makeCallback(resolve, reject, (result: any) => {
const headers: any = fetchMock.lastCall()![1]!.headers;
const headers: Record<string, string> = fetchMock.lastCall()![1]!.headers as Record<string, string>;
expect(headers.authorization).toBe('1234');
expect(headers['content-type']).toBe('application/json');
expect(headers.accept).toBe('*/*');
Expand Down
1 change: 1 addition & 0 deletions src/link/http/createHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export const createHttpLink = (linkOptions: HttpOptions = {}) => {

// does not match custom directives beginning with @defer
if (hasDirectives(['defer'], operation.query)) {
options.headers = options.headers || {};
options.headers.accept = "multipart/mixed; deferSpec=20220824, application/json";
}

Expand Down
18 changes: 10 additions & 8 deletions src/link/http/selectHttpOptionsAndBody.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ export interface HttpOptions {
/**
* An object representing values to be sent as headers on the request.
*/
headers?: any;
headers?: Record<string, string>;

/**
* If set to true, header names won't be automatically normalized to
* lowercase. This allows for non-http-spec-compliant servers that might
* If set to true, header names won't be automatically normalized to
* lowercase. This allows for non-http-spec-compliant servers that might
* expect capitalized header names.
*/
preserveHeaderCase?: boolean;
Expand Down Expand Up @@ -92,7 +92,7 @@ export interface HttpQueryOptions {
export interface HttpConfig {
http?: HttpQueryOptions;
options?: any;
headers?: any;
headers?: Record<string, string>;
credentials?: any;
}

Expand Down Expand Up @@ -173,7 +173,9 @@ export function selectHttpOptionsAndBodyInternal(
};
});

options.headers = removeDuplicateHeaders(options.headers, http.preserveHeaderCase);
if (options.headers) {
options.headers = removeDuplicateHeaders(options.headers, http.preserveHeaderCase);
}

//The body depends on the http options
const { operationName, extensions, variables, query } = operation;
Expand All @@ -191,7 +193,7 @@ export function selectHttpOptionsAndBodyInternal(
};

// Remove potential duplicate header names, preserving last (by insertion order).
// This is done to prevent unintentionally duplicating a header instead of
// This is done to prevent unintentionally duplicating a header instead of
// overwriting it (See #8447 and #8449).
function removeDuplicateHeaders(
headers: Record<string, string>,
Expand All @@ -204,12 +206,12 @@ function removeDuplicateHeaders(
Object.keys(Object(headers)).forEach(name => {
normalizedHeaders[name.toLowerCase()] = headers[name];
});
return normalizedHeaders;
return normalizedHeaders;
}

// If we are preserving the case, remove duplicates w/ normalization,
// preserving the original name.
// This allows for non-http-spec-compliant servers that expect intentionally
// This allows for non-http-spec-compliant servers that expect intentionally
// capitalized header names (See #6741).
const headerData = Object.create(null);
Object.keys(Object(headers)).forEach(name => {
Expand Down

0 comments on commit 895ddcb

Please sign in to comment.