From 93fce5f2af517a456bbca285761dff525caeea8d Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Tue, 12 Oct 2021 14:54:43 +0300 Subject: [PATCH] GraphQLError: use enumerable properties Since we now provide `toJSON` method it means it's finally safe to use enumerable properties and convert class into idiomatic TS. Previously TS didn't check `Object.defineProperties` so we could only only define optional properties on this class. --- src/error/GraphQLError.ts | 117 +++++++++++--------------------------- 1 file changed, 34 insertions(+), 83 deletions(-) diff --git a/src/error/GraphQLError.ts b/src/error/GraphQLError.ts index 2bea7d4103..80bb9fcba3 100644 --- a/src/error/GraphQLError.ts +++ b/src/error/GraphQLError.ts @@ -24,7 +24,7 @@ export class GraphQLError extends Error { * * Enumerable, and appears in the result of JSON.stringify(). */ - readonly locations?: ReadonlyArray; + readonly locations: ReadonlyArray | undefined; /** * An array describing the JSON-path into the execution response which @@ -32,12 +32,12 @@ export class GraphQLError extends Error { * * Enumerable, and appears in the result of JSON.stringify(). */ - readonly path?: ReadonlyArray; + readonly path: ReadonlyArray | undefined; /** * An array of GraphQL AST Nodes corresponding to this error. */ - readonly nodes?: ReadonlyArray; + readonly nodes: ReadonlyArray | undefined; /** * The source GraphQL document for the first location of this error. @@ -45,23 +45,23 @@ export class GraphQLError extends Error { * Note that if this Error represents more than one node, the source may not * represent nodes after the first node. */ - readonly source?: Source; + readonly source: Source | undefined; /** * An array of character offsets within the source GraphQL document * which correspond to this error. */ - readonly positions?: ReadonlyArray; + readonly positions: ReadonlyArray | undefined; /** * The original error thrown from a field resolver during execution. */ - readonly originalError: Maybe; + readonly originalError: Error | undefined; /** * Extension fields to add to the formatted error. */ - readonly extensions?: { [key: string]: unknown }; + readonly extensions: { [key: string]: unknown } | undefined; constructor( message: string, @@ -74,8 +74,12 @@ export class GraphQLError extends Error { ) { super(message); + this.name = 'GraphQLError'; + this.path = path ?? undefined; + this.originalError = originalError ?? undefined; + // Compute list of blame nodes. - const _nodes = Array.isArray(nodes) + this.nodes = Array.isArray(nodes) ? nodes.length !== 0 ? nodes : undefined @@ -84,96 +88,43 @@ export class GraphQLError extends Error { : undefined; // Compute locations in the source for the given nodes/positions. - let _source = source; - if (!_source && _nodes) { - _source = _nodes[0].loc?.source; + this.source = source ?? undefined; + if (!this.source && this.nodes) { + this.source = this.nodes[0].loc?.source; } - let _positions; if (positions) { - _positions = positions; - } else if (_nodes) { - _positions = []; - for (const node of _nodes) { + this.positions = positions; + } else if (this.nodes) { + const positionsFromNodes = []; + for (const node of this.nodes) { if (node.loc) { - _positions.push(node.loc.start); + positionsFromNodes.push(node.loc.start); } } + this.positions = positionsFromNodes; } - if (_positions && _positions.length === 0) { - _positions = undefined; + if (this.positions && this.positions.length === 0) { + this.positions = undefined; } - let _locations; if (positions && source) { - _locations = positions.map((pos) => getLocation(source, pos)); - } else if (_nodes) { - _locations = []; - for (const node of _nodes) { + this.locations = positions.map((pos) => getLocation(source, pos)); + } else if (this.nodes) { + const locationsFromNodes = []; + for (const node of this.nodes) { if (node.loc) { - _locations.push(getLocation(node.loc.source, node.loc.start)); + locationsFromNodes.push(getLocation(node.loc.source, node.loc.start)); } } + this.locations = locationsFromNodes; } - let _extensions = extensions; - if (_extensions == null && originalError != null) { - const originalExtensions = originalError.extensions; - if (isObjectLike(originalExtensions)) { - _extensions = originalExtensions; - } - } - - Object.defineProperties(this, { - name: { value: 'GraphQLError' }, - message: { - value: message, - // By being enumerable, JSON.stringify will include `message` in the - // resulting output. This ensures that the simplest possible GraphQL - // service adheres to the spec. - enumerable: true, - writable: true, - }, - locations: { - // Coercing falsy values to undefined ensures they will not be included - // in JSON.stringify() when not provided. - value: _locations ?? undefined, - // By being enumerable, JSON.stringify will include `locations` in the - // resulting output. This ensures that the simplest possible GraphQL - // service adheres to the spec. - enumerable: _locations != null, - }, - path: { - // Coercing falsy values to undefined ensures they will not be included - // in JSON.stringify() when not provided. - value: path ?? undefined, - // By being enumerable, JSON.stringify will include `path` in the - // resulting output. This ensures that the simplest possible GraphQL - // service adheres to the spec. - enumerable: path != null, - }, - nodes: { - value: _nodes ?? undefined, - }, - source: { - value: _source ?? undefined, - }, - positions: { - value: _positions ?? undefined, - }, - originalError: { - value: originalError, - }, - extensions: { - // Coercing falsy values to undefined ensures they will not be included - // in JSON.stringify() when not provided. - value: _extensions ?? undefined, - // By being enumerable, JSON.stringify will include `path` in the - // resulting output. This ensures that the simplest possible GraphQL - // service adheres to the spec. - enumerable: _extensions != null, - }, - }); + const originalExtensions = isObjectLike(originalError?.extensions) + ? originalError?.extensions + : undefined; + // TODO: merge `extensions` and `originalExtensions` + this.extensions = extensions ?? originalExtensions; // Include (non-enumerable) stack trace. if (originalError?.stack) {