Skip to content

Commit

Permalink
Add arbitrary property to HeaderMap for type differentiation (#7314)
Browse files Browse the repository at this point in the history
Related: #7313

This shouldn't have been allowed by the typings in the first place.
Previously, a `Map` was compatible with `HeaderMap` according to
TypeScript, so the error wasn't a typing error (when we wanted it to
be). Adding an arbitrary class property to the `HeaderMap` class gives
us the typing error we'd expect when a `Map` is used in lieu of a
`HeaderMap`.
  • Loading branch information
trevor-scheer committed Jan 19, 2023
1 parent f329139 commit f246ddb
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
12 changes: 12 additions & 0 deletions .changeset/great-kids-visit.md
@@ -0,0 +1,12 @@
---
'@apollo/server': patch
---

Add an `__identity` property to `HeaderMap` class to disallow standard `Map`s (in TypeScript).

This ensures that typechecking occurs on fields which are declared to accept a
`HeaderMap` (notably, the `httpGraphQLRequest.headers` option to
`ApolloServer.executeHTTPGraphQLRequest` and the `http.headers` option to
`ApolloServer.executeOperation`). This might be a breaking change for
integration authors, but should be easily fixed by switching from `new
Map<string, string>()` to `new HeaderMap()`.
8 changes: 5 additions & 3 deletions docs/source/integrations/building-integrations.md
Expand Up @@ -149,7 +149,7 @@ With the request body parsed, we can now construct an `HTTPGraphQLRequest`:
```ts
interface HTTPGraphQLRequest {
method: string;
headers: Map<string, string>;
headers: HeaderMap; // the `HeaderMap` class is exported by @apollo/server
search: string;
body: unknown;
}
Expand All @@ -164,7 +164,9 @@ Finally, we have to create the `headers` property because Apollo Server expects
In the Express integration, we construct a `Map` by iterating over the `headers` object, like so:

```ts
const headers = new Map<string, string>();
import { HeaderMap } from '@apollo/server';

const headers = new HeaderMap();
for (const [key, value] of Object.entries(req.headers)) {
if (value !== undefined) {
headers.set(key, Array.isArray(value) ? value.join(', ') : value);
Expand Down Expand Up @@ -221,7 +223,7 @@ After awaiting the Promise returned by `executeHTTPGraphQLRequest`, we receive a
```ts
interface HTTPGraphQLHead {
status?: number;
headers: Map<string, string>;
headers: HeaderMap;
}

type HTTPGraphQLResponseBody =
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/__tests__/runQuery.test.ts
Expand Up @@ -837,7 +837,7 @@ describe('request pipeline life-cycle hooks', () => {
message: 'Syntax Error: Expected Name, found "}".',
extensions: {
code: 'GRAPHQL_PARSE_FAILED',
http: { status: 400, headers: new HeaderMap() },
http: { status: 400, headers: expect.any(HeaderMap) },
},
}),
]),
Expand All @@ -862,7 +862,7 @@ describe('request pipeline life-cycle hooks', () => {
'Cannot query field "testStringWithParseError" on type "QueryType".',
extensions: {
code: 'GRAPHQL_VALIDATION_FAILED',
http: { status: 400, headers: new HeaderMap() },
http: { status: 400, headers: expect.any(HeaderMap) },
},
}),
]),
Expand Down
5 changes: 5 additions & 0 deletions packages/server/src/utils/HeaderMap.ts
@@ -1,4 +1,9 @@
export class HeaderMap extends Map<string, string> {
// In order for TypeScript to prevent a standard `Map` from being compatible
// with a `HeaderMap`, we need some additional property on the class.
// @ts-ignore (this is just unused)
private __identity = Symbol('HeaderMap');

override set(key: string, value: string): this {
return super.set(key.toLowerCase(), value);
}
Expand Down

0 comments on commit f246ddb

Please sign in to comment.