From 9a40a9aa565f4b778b0f1cc920e1c52cb85f3868 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Sat, 12 Dec 2020 13:22:00 +0300 Subject: [PATCH 1/3] Allow custom graphql params extraction from request In my app I'm trying to move away from express. I implemented own body parser to not pollute `request` object. Though in this case express-graphql is trying to parse body by itself which leads to hanging server because there are no more events on request. In this diff I added a way to custom extracting params from request. It would look better in options though options getter depends on parsed options. --- package-lock.json | 72 -------------------------------------- src/__tests__/http-test.ts | 30 ++++++++++++++++ src/index.ts | 14 ++++++-- 3 files changed, 42 insertions(+), 74 deletions(-) diff --git a/package-lock.json b/package-lock.json index 001f567c..928abda2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1003,41 +1003,6 @@ "integrity": "sha512-+q/t7Ekv1EDY2l6Gda6LLiX14rU9TV20Wa3ofeQmwPFZbOMo9DXrLbOjFaaclkXKWidIaopwAObQDqwWtGUjqg==", "dev": true }, - "babel-plugin-emotion": { - "version": "10.0.33", - "resolved": "https://registry.npmjs.org/babel-plugin-emotion/-/babel-plugin-emotion-10.0.33.tgz", - "integrity": "sha512-bxZbTTGz0AJQDHm8k6Rf3RQJ8tX2scsfsRyKVgAbiUPUNIRtlK+7JxP+TAd1kRLABFxe0CFm2VdK4ePkoA9FxQ==", - "dev": true, - "requires": { - "@babel/helper-module-imports": "^7.0.0", - "@emotion/hash": "0.8.0", - "@emotion/memoize": "0.7.4", - "@emotion/serialize": "^0.11.16", - "babel-plugin-macros": "^2.0.0", - "babel-plugin-syntax-jsx": "^6.18.0", - "convert-source-map": "^1.5.0", - "escape-string-regexp": "^1.0.5", - "find-root": "^1.1.0", - "source-map": "^0.5.7" - } - }, - "babel-plugin-macros": { - "version": "2.8.0", - "resolved": "https://registry.npmjs.org/babel-plugin-macros/-/babel-plugin-macros-2.8.0.tgz", - "integrity": "sha512-SEP5kJpfGYqYKpBrj5XU3ahw5p5GOHJ0U5ssOSQ/WBVdwkD2Dzlce95exQTs3jOVWPPKLBN2rlEWkCK7dSmLvg==", - "dev": true, - "requires": { - "@babel/runtime": "^7.7.2", - "cosmiconfig": "^6.0.0", - "resolve": "^1.12.0" - } - }, - "babel-plugin-syntax-jsx": { - "version": "6.18.0", - "resolved": "https://registry.npmjs.org/babel-plugin-syntax-jsx/-/babel-plugin-syntax-jsx-6.18.0.tgz", - "integrity": "sha1-CvMqmm4Tyno/1QaeYtew9Y0NiUY=", - "dev": true - }, "backo2": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/backo2/-/backo2-1.0.2.tgz", @@ -3608,22 +3573,6 @@ "integrity": "sha512-QZ9qOMdF+QLHxy1QIpUHUU1D5pS2CG2P69LF6L6CPjPYA/XMOmKV3PZpawHoAjHNyB0swdVTRxdYT4tbBbxqwg==", "dev": true }, - "iterate-iterator": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/iterate-iterator/-/iterate-iterator-1.0.1.tgz", - "integrity": "sha512-3Q6tudGN05kbkDQDI4CqjaBf4qf85w6W6GnuZDtUVYwKgtC1q8yxYX7CZed7N+tLzQqS6roujWvszf13T+n9aw==", - "dev": true - }, - "iterate-value": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/iterate-value/-/iterate-value-1.0.2.tgz", - "integrity": "sha512-A6fMAio4D2ot2r/TYzr4yUWrmwNdsN5xL7+HUiyACE4DXm+q8HtPcnFTp+NnW3k4N05tZ7FVYFFb2CR13NxyHQ==", - "dev": true, - "requires": { - "es-get-iterator": "^1.0.2", - "iterate-iterator": "^1.0.1" - } - }, "js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -5571,27 +5520,6 @@ "integrity": "sha512-6fPc+R4ihwqP6N/aIv2f1gMH8lOVtWQHoqC4yK6oSDVVocumAsfCqjkXnqiYMhmMwS/mEHLp7Vehlt3ql6lEig==", "dev": true }, - "styled-system": { - "version": "5.1.5", - "resolved": "https://registry.npmjs.org/styled-system/-/styled-system-5.1.5.tgz", - "integrity": "sha512-7VoD0o2R3RKzOzPK0jYrVnS8iJdfkKsQJNiLRDjikOpQVqQHns/DXWaPZOH4tIKkhAT7I6wIsy9FWTWh2X3q+A==", - "dev": true, - "requires": { - "@styled-system/background": "^5.1.2", - "@styled-system/border": "^5.1.5", - "@styled-system/color": "^5.1.2", - "@styled-system/core": "^5.1.2", - "@styled-system/flexbox": "^5.1.2", - "@styled-system/grid": "^5.1.2", - "@styled-system/layout": "^5.1.2", - "@styled-system/position": "^5.1.2", - "@styled-system/shadow": "^5.1.2", - "@styled-system/space": "^5.1.2", - "@styled-system/typography": "^5.1.2", - "@styled-system/variant": "^5.1.5", - "object-assign": "^4.1.1" - } - }, "subscriptions-transport-ws": { "version": "0.5.4", "resolved": "https://registry.npmjs.org/subscriptions-transport-ws/-/subscriptions-transport-ws-0.5.4.tgz", diff --git a/src/__tests__/http-test.ts b/src/__tests__/http-test.ts index 6a2b3218..a6ca4909 100644 --- a/src/__tests__/http-test.ts +++ b/src/__tests__/http-test.ts @@ -1029,6 +1029,36 @@ function runTests(server: Server) { }); }); + it('allow custom graphql params extraction from request', async () => { + const app = server(); + + app.get( + urlString(), + graphqlHTTP( + { + schema: TestSchema, + }, + (request) => { + const searchParams = new URLSearchParams(request.url.split('?')[1]); + return { + query: searchParams.get('graphqlQuery'), + variables: null, + operationName: null, + raw: false, + }; + }, + ), + ); + + const response = await app.request().get( + urlString({ + graphqlQuery: '{test}', + }), + ); + + expect(response.text).to.equal('{"data":{"test":"Hello World"}}'); + }); + describe('Pretty printing', () => { it('supports pretty printing', async () => { const app = server(); diff --git a/src/index.ts b/src/index.ts index b04fdb06..9588ccaa 100644 --- a/src/index.ts +++ b/src/index.ts @@ -195,7 +195,16 @@ type Middleware = (request: Request, response: Response) => Promise; * Middleware for express; takes an options object or function as input to * configure behavior, and returns an express middleware. */ -export function graphqlHTTP(options: Options): Middleware { +export function graphqlHTTP( + options: Options, + /** + * An optional function which will get params from request instead of + * default getGraphQLParams + */ + customGraphQLParamsFn?: ( + request: Request, + ) => GraphQLParams | Promise, +): Middleware { devAssert(options != null, 'GraphQL middleware requires options.'); return async function graphqlMiddleware( @@ -210,11 +219,12 @@ export function graphqlHTTP(options: Options): Middleware { let pretty = false; let result: ExecutionResult; let optionsData: OptionsData | undefined; + const graphQLParamsFn = customGraphQLParamsFn ?? getGraphQLParams; try { // Parse the Request to get GraphQL request parameters. try { - params = await getGraphQLParams(request); + params = await graphQLParamsFn(request); } catch (error: unknown) { // When we failed to parse the GraphQL parameters, we still need to get // the options object, so make an options call to resolve just that. From cb8b0eab204b7496843ceaa2fedeb04fffded542 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 6 Jan 2021 17:16:40 +0300 Subject: [PATCH 2/3] Move customGraphQLParamsFn to options --- src/__tests__/http-test.ts | 10 ++--- src/index.ts | 88 ++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/__tests__/http-test.ts b/src/__tests__/http-test.ts index a6ca4909..c92b5f26 100644 --- a/src/__tests__/http-test.ts +++ b/src/__tests__/http-test.ts @@ -1034,11 +1034,9 @@ function runTests(server: Server) { app.get( urlString(), - graphqlHTTP( - { - schema: TestSchema, - }, - (request) => { + graphqlHTTP({ + schema: TestSchema, + customGraphQLParamsFn: (request) => { const searchParams = new URLSearchParams(request.url.split('?')[1]); return { query: searchParams.get('graphqlQuery'), @@ -1047,7 +1045,7 @@ function runTests(server: Server) { raw: false, }; }, - ), + }), ); const response = await app.request().get( diff --git a/src/index.ts b/src/index.ts index 9588ccaa..688c6fca 100644 --- a/src/index.ts +++ b/src/index.ts @@ -109,6 +109,14 @@ export interface OptionsData { */ customParseFn?: (source: Source) => DocumentNode; + /** + * An optional function which will get params from request instead of + * default getGraphQLParams + */ + customGraphQLParamsFn?: ( + request: Request, + ) => GraphQLParams | Promise; + /** * `formatError` is deprecated and replaced by `customFormatErrorFn`. It will * be removed in version 1.0.0. @@ -195,16 +203,7 @@ type Middleware = (request: Request, response: Response) => Promise; * Middleware for express; takes an options object or function as input to * configure behavior, and returns an express middleware. */ -export function graphqlHTTP( - options: Options, - /** - * An optional function which will get params from request instead of - * default getGraphQLParams - */ - customGraphQLParamsFn?: ( - request: Request, - ) => GraphQLParams | Promise, -): Middleware { +export function graphqlHTTP(options: Options): Middleware { devAssert(options != null, 'GraphQL middleware requires options.'); return async function graphqlMiddleware( @@ -215,30 +214,34 @@ export function graphqlHTTP( let params: GraphQLParams | undefined; let showGraphiQL = false; let graphiqlOptions; - let formatErrorFn = formatError; - let pretty = false; let result: ExecutionResult; let optionsData: OptionsData | undefined; - const graphQLParamsFn = customGraphQLParamsFn ?? getGraphQLParams; try { - // Parse the Request to get GraphQL request parameters. - try { - params = await graphQLParamsFn(request); - } catch (error: unknown) { - // When we failed to parse the GraphQL parameters, we still need to get - // the options object, so make an options call to resolve just that. - optionsData = await resolveOptions(); - pretty = optionsData.pretty ?? false; - formatErrorFn = - optionsData.customFormatErrorFn ?? - optionsData.formatError ?? - formatErrorFn; - throw error; + if (typeof options === 'function') { + try { + // Parse the Request to get GraphQL request parameters. + params = await getGraphQLParams(request); + } catch (error: unknown) { + // When we failed to parse the GraphQL parameters, we still need to get + // the options object, so make an options call to resolve just that. + optionsData = await options(request, response); + validateOptions(); + throw error; + } + optionsData = await options(request, response, params); + validateOptions(); + } else { + optionsData = await options; + validateOptions(); } - // Then, resolve the Options to get OptionsData. - optionsData = await resolveOptions(params); + if (optionsData.customGraphQLParamsFn) { + params = await optionsData.customGraphQLParamsFn(request); + } else if (!params) { + // Parse the Request to get GraphQL request parameters. + params = await getGraphQLParams(request); + } // Collect information from the options data object. const schema = optionsData.schema; @@ -253,12 +256,6 @@ export function graphqlHTTP( const executeFn = optionsData.customExecuteFn ?? execute; const validateFn = optionsData.customValidateFn ?? validate; - pretty = optionsData.pretty ?? false; - formatErrorFn = - optionsData.customFormatErrorFn ?? - optionsData.formatError ?? - formatErrorFn; - // Assert that schema is required. devAssert( schema != null, @@ -384,6 +381,7 @@ export function graphqlHTTP( rawError instanceof Error ? rawError : String(rawError), ); + // eslint-disable-next-line require-atomic-updates response.statusCode = error.status; const { headers } = error; @@ -408,6 +406,12 @@ export function graphqlHTTP( } } + const pretty = optionsData?.pretty ?? false; + const formatErrorFn = + optionsData?.customFormatErrorFn ?? + optionsData?.formatError ?? + formatError; + if (result.errors != null || result.data == null) { const handleRuntimeQueryErrorFn = optionsData?.handleRuntimeQueryErrorFn ?? @@ -452,28 +456,18 @@ export function graphqlHTTP( sendResponse(response, 'application/json', payload); } - async function resolveOptions( - requestParams?: GraphQLParams, - ): Promise { - const optionsResult = await Promise.resolve( - typeof options === 'function' - ? options(request, response, requestParams) - : options, - ); - + function validateOptions() { devAssert( - optionsResult != null && typeof optionsResult === 'object', + optionsData != null && typeof optionsData === 'object', 'GraphQL middleware option function must return an options object or a promise which will be resolved to an options object.', ); - if (optionsResult.formatError) { + if (optionsData.formatError) { // eslint-disable-next-line no-console console.warn( '`formatError` is deprecated and replaced by `customFormatErrorFn`. It will be removed in version 1.0.0.', ); } - - return optionsResult; } }; } From ab89b1c5b3b6af41bc1e06fa8575f53d0504bb89 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 6 Jan 2021 17:20:47 +0300 Subject: [PATCH 3/3] Revert lockfile --- package-lock.json | 72 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/package-lock.json b/package-lock.json index 928abda2..001f567c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1003,6 +1003,41 @@ "integrity": "sha512-+q/t7Ekv1EDY2l6Gda6LLiX14rU9TV20Wa3ofeQmwPFZbOMo9DXrLbOjFaaclkXKWidIaopwAObQDqwWtGUjqg==", "dev": true }, + "babel-plugin-emotion": { + "version": "10.0.33", + "resolved": "https://registry.npmjs.org/babel-plugin-emotion/-/babel-plugin-emotion-10.0.33.tgz", + "integrity": "sha512-bxZbTTGz0AJQDHm8k6Rf3RQJ8tX2scsfsRyKVgAbiUPUNIRtlK+7JxP+TAd1kRLABFxe0CFm2VdK4ePkoA9FxQ==", + "dev": true, + "requires": { + "@babel/helper-module-imports": "^7.0.0", + "@emotion/hash": "0.8.0", + "@emotion/memoize": "0.7.4", + "@emotion/serialize": "^0.11.16", + "babel-plugin-macros": "^2.0.0", + "babel-plugin-syntax-jsx": "^6.18.0", + "convert-source-map": "^1.5.0", + "escape-string-regexp": "^1.0.5", + "find-root": "^1.1.0", + "source-map": "^0.5.7" + } + }, + "babel-plugin-macros": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/babel-plugin-macros/-/babel-plugin-macros-2.8.0.tgz", + "integrity": "sha512-SEP5kJpfGYqYKpBrj5XU3ahw5p5GOHJ0U5ssOSQ/WBVdwkD2Dzlce95exQTs3jOVWPPKLBN2rlEWkCK7dSmLvg==", + "dev": true, + "requires": { + "@babel/runtime": "^7.7.2", + "cosmiconfig": "^6.0.0", + "resolve": "^1.12.0" + } + }, + "babel-plugin-syntax-jsx": { + "version": "6.18.0", + "resolved": "https://registry.npmjs.org/babel-plugin-syntax-jsx/-/babel-plugin-syntax-jsx-6.18.0.tgz", + "integrity": "sha1-CvMqmm4Tyno/1QaeYtew9Y0NiUY=", + "dev": true + }, "backo2": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/backo2/-/backo2-1.0.2.tgz", @@ -3573,6 +3608,22 @@ "integrity": "sha512-QZ9qOMdF+QLHxy1QIpUHUU1D5pS2CG2P69LF6L6CPjPYA/XMOmKV3PZpawHoAjHNyB0swdVTRxdYT4tbBbxqwg==", "dev": true }, + "iterate-iterator": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/iterate-iterator/-/iterate-iterator-1.0.1.tgz", + "integrity": "sha512-3Q6tudGN05kbkDQDI4CqjaBf4qf85w6W6GnuZDtUVYwKgtC1q8yxYX7CZed7N+tLzQqS6roujWvszf13T+n9aw==", + "dev": true + }, + "iterate-value": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/iterate-value/-/iterate-value-1.0.2.tgz", + "integrity": "sha512-A6fMAio4D2ot2r/TYzr4yUWrmwNdsN5xL7+HUiyACE4DXm+q8HtPcnFTp+NnW3k4N05tZ7FVYFFb2CR13NxyHQ==", + "dev": true, + "requires": { + "es-get-iterator": "^1.0.2", + "iterate-iterator": "^1.0.1" + } + }, "js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -5520,6 +5571,27 @@ "integrity": "sha512-6fPc+R4ihwqP6N/aIv2f1gMH8lOVtWQHoqC4yK6oSDVVocumAsfCqjkXnqiYMhmMwS/mEHLp7Vehlt3ql6lEig==", "dev": true }, + "styled-system": { + "version": "5.1.5", + "resolved": "https://registry.npmjs.org/styled-system/-/styled-system-5.1.5.tgz", + "integrity": "sha512-7VoD0o2R3RKzOzPK0jYrVnS8iJdfkKsQJNiLRDjikOpQVqQHns/DXWaPZOH4tIKkhAT7I6wIsy9FWTWh2X3q+A==", + "dev": true, + "requires": { + "@styled-system/background": "^5.1.2", + "@styled-system/border": "^5.1.5", + "@styled-system/color": "^5.1.2", + "@styled-system/core": "^5.1.2", + "@styled-system/flexbox": "^5.1.2", + "@styled-system/grid": "^5.1.2", + "@styled-system/layout": "^5.1.2", + "@styled-system/position": "^5.1.2", + "@styled-system/shadow": "^5.1.2", + "@styled-system/space": "^5.1.2", + "@styled-system/typography": "^5.1.2", + "@styled-system/variant": "^5.1.5", + "object-assign": "^4.1.1" + } + }, "subscriptions-transport-ws": { "version": "0.5.4", "resolved": "https://registry.npmjs.org/subscriptions-transport-ws/-/subscriptions-transport-ws-0.5.4.tgz",