Skip to content

Commit

Permalink
feat: Subscribe message query must be a string (enisdenjo#45)
Browse files Browse the repository at this point in the history
* feat: query can be only of string type

* docs: update apollo example

* docs(protocol): no document node
  • Loading branch information
enisdenjo committed Nov 3, 2020
1 parent 9740774 commit 60d9cd5
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 72 deletions.
4 changes: 1 addition & 3 deletions PROTOCOL.md
Expand Up @@ -62,14 +62,12 @@ Requests an operation specified in the message `payload`. This message provides
If there is already an active subscriber for a streaming operation matching the provided ID, the server will close the socket immediately with the event `4409: Subscriber for <unique-operation-id> already exists`. The server may not assert this rule for operations returning a single result as they do not require reservations for additional future events.

```typescript
import { DocumentNode } from 'graphql';

interface SubscribeMessage {
id: '<unique-operation-id>';
type: 'subscribe';
payload: {
operationName?: string | null;
query: string | DocumentNode;
query: string;
variables?: Record<string, unknown> | null;
};
}
Expand Down
42 changes: 24 additions & 18 deletions README.md
Expand Up @@ -272,6 +272,7 @@ export const network = Network.create(fetchOrSubscribe, fetchOrSubscribe);

```typescript
import { ApolloLink, Operation, FetchResult, Observable } from '@apollo/client';
import { print } from 'graphql';
import { createClient, Config, Client } from 'graphql-ws';

class WebSocketLink extends ApolloLink {
Expand All @@ -284,25 +285,30 @@ class WebSocketLink extends ApolloLink {

public request(operation: Operation): Observable<FetchResult> {
return new Observable((sink) => {
return this.client.subscribe<FetchResult>(operation, {
...sink,
error: (err) => {
if (err instanceof Error) {
sink.error(err);
} else if (err instanceof CloseEvent) {
sink.error(
new Error(
`Socket closed with event ${err.code}` + err.reason
? `: ${err.reason}` // reason will be available on clean closes
: '',
),
);
} else {
// GraphQLError[]
sink.error(new Error(err.map(({ message }) => message).join(', ')));
}
return this.client.subscribe<FetchResult>(
{ ...operation, query: print(operation.query) },
{
...sink,
error: (err) => {
if (err instanceof Error) {
sink.error(err);
} else if (err instanceof CloseEvent) {
sink.error(
new Error(
`Socket closed with event ${err.code}` + err.reason
? `: ${err.reason}` // reason will be available on clean closes
: '',
),
);
} else {
// GraphQLError[]
sink.error(
new Error(err.map(({ message }) => message).join(', ')),
);
}
},
},
});
);
});
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/message.ts
Expand Up @@ -4,7 +4,7 @@
*
*/

import { GraphQLError, ExecutionResult, DocumentNode } from 'graphql';
import { GraphQLError, ExecutionResult } from 'graphql';
import {
isObject,
areGraphQLErrors,
Expand Down Expand Up @@ -41,7 +41,7 @@ export interface SubscribeMessage {

export interface SubscribePayload {
readonly operationName?: string | null;
readonly query: string | DocumentNode;
readonly query: string;
readonly variables?: Record<string, unknown> | null;
}

Expand Down Expand Up @@ -104,8 +104,7 @@ export function isMessage(val: unknown): val is Message {
val.payload.operationName === undefined ||
val.payload.operationName === null ||
typeof val.payload.operationName === 'string') &&
(hasOwnStringProperty(val.payload, 'query') || // string query or persisted query id
hasOwnObjectProperty(val.payload, 'query')) && // document node query
hasOwnStringProperty(val.payload, 'query') &&
(!hasOwnProperty(val.payload, 'variables') ||
val.payload.variables === undefined ||
val.payload.variables === null ||
Expand Down
3 changes: 1 addition & 2 deletions src/server.ts
Expand Up @@ -567,11 +567,10 @@ export function createServer(
}

const { operationName, query, variables } = message.payload;
const document = typeof query === 'string' ? parse(query) : query;
execArgs = {
schema,
operationName,
document,
document: parse(query),
variableValues: variables,
};

Expand Down
45 changes: 0 additions & 45 deletions src/tests/server.ts
Expand Up @@ -1164,51 +1164,6 @@ describe('Subscribe', () => {
});
});

it('should execute the query of `DocumentNode` type, "next" the result and then "complete"', async () => {
const { url } = await startTServer({
schema,
});

const client = await createTClient(url);
client.ws.send(
stringifyMessage<MessageType.ConnectionInit>({
type: MessageType.ConnectionInit,
}),
);

await client.waitForMessage(({ data }) => {
expect(parseMessage(data).type).toBe(MessageType.ConnectionAck);
client.ws.send(
stringifyMessage<MessageType.Subscribe>({
id: '1',
type: MessageType.Subscribe,
payload: {
operationName: 'TestString',
query: parse(`query TestString {
getValue
}`),
variables: {},
},
}),
);
});

await client.waitForMessage(({ data }) => {
expect(parseMessage(data)).toEqual({
id: '1',
type: MessageType.Next,
payload: { data: { getValue: 'value' } },
});
});

await client.waitForMessage(({ data }) => {
expect(parseMessage(data)).toEqual({
id: '1',
type: MessageType.Complete,
});
});
});

it('should execute the query and "error" out because of validation errors', async () => {
const { url } = await startTServer({
schema,
Expand Down

0 comments on commit 60d9cd5

Please sign in to comment.