Skip to content

Commit

Permalink
Support symbol properties on extension objects (#4234)
Browse files Browse the repository at this point in the history
Supersedes graphql/graphql-js#3511

This adds support for `Symbol` property keys on the extension property
of AST nodes. Had to tweak `toObjMap` here to support symbol keys by
adding `getOwnPropertySymbols` which is the only way to enumerate over
them.

---------

Co-authored-by: n1ru4l <laurinquast@googlemail.com>
  • Loading branch information
2 people authored and erikwrede committed Oct 17, 2024
1 parent dd1ed94 commit 8d99fa4
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 28 deletions.
8 changes: 8 additions & 0 deletions src/jsutils/ObjMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ export interface ReadOnlyObjMap<T> {
readonly [key: string]: T;
}

export interface ReadOnlyObjMapWithSymbol<T> {
readonly [key: string | symbol]: T;
}

export type ReadOnlyObjMapLike<T> =
| ReadOnlyObjMap<T>
| { readonly [key: string]: T };

export type ReadOnlyObjMapSymbolLike<T> =
| ReadOnlyObjMapWithSymbol<T>
| { readonly [key: string | symbol]: T };
31 changes: 30 additions & 1 deletion src/jsutils/toObjMap.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import type { Maybe } from './Maybe.js';
import type { ReadOnlyObjMap, ReadOnlyObjMapLike } from './ObjMap.js';
import type {
ReadOnlyObjMap,
ReadOnlyObjMapLike,
ReadOnlyObjMapSymbolLike,
ReadOnlyObjMapWithSymbol,
} from './ObjMap.js';

export function toObjMap<T>(
obj: Maybe<ReadOnlyObjMapLike<T>>,
Expand All @@ -16,5 +21,29 @@ export function toObjMap<T>(
for (const [key, value] of Object.entries(obj)) {
map[key] = value;
}

return map;
}

export function toObjMapWithSymbols<T>(
obj: Maybe<ReadOnlyObjMapSymbolLike<T>>,
): ReadOnlyObjMapWithSymbol<T> {
if (obj == null) {
return Object.create(null);
}

if (Object.getPrototypeOf(obj) === null) {
return obj;
}

const map = Object.create(null);
for (const [key, value] of Object.entries(obj)) {
map[key] = value;
}

for (const key of Object.getOwnPropertySymbols(obj)) {
map[key] = obj[key];
}

return map;
}
19 changes: 19 additions & 0 deletions src/type/__tests__/definition-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,25 @@ describe('Type System: Scalars', () => {
expect(someScalar.toConfig()).to.deep.equal(someScalarConfig);
});

it('supports symbol extensions', () => {
const test = Symbol.for('test');
const someScalarConfig: GraphQLScalarTypeConfig<unknown, unknown> = {
name: 'SomeScalar',
description: 'SomeScalar description.',
specifiedByURL: 'https://example.com/foo_spec',
serialize: passThroughFunc,
parseValue: passThroughFunc,
parseLiteral: passThroughFunc,
coerceInputLiteral: passThroughFunc,
valueToLiteral: passThroughFunc,
extensions: { [test]: 'extension' },
astNode: dummyAny,
extensionASTNodes: [dummyAny],
};
const someScalar = new GraphQLScalarType(someScalarConfig);
expect(someScalar.toConfig()).to.deep.equal(someScalarConfig);
});

it('provides default methods if omitted', () => {
const scalar = new GraphQLScalarType({ name: 'Foo' });

Expand Down
42 changes: 21 additions & 21 deletions src/type/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { ObjMap } from '../jsutils/ObjMap.js';
import type { Path } from '../jsutils/Path.js';
import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js';
import { suggestionList } from '../jsutils/suggestionList.js';
import { toObjMap } from '../jsutils/toObjMap.js';
import { toObjMapWithSymbols } from '../jsutils/toObjMap.js';

import { GraphQLError } from '../error/GraphQLError.js';

Expand Down Expand Up @@ -511,7 +511,7 @@ export function resolveObjMapThunk<T>(thunk: ThunkObjMap<T>): ObjMap<T> {
* an object which can contain all the values you need.
*/
export interface GraphQLScalarTypeExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand Down Expand Up @@ -610,7 +610,7 @@ export class GraphQLScalarType<TInternal = unknown, TExternal = TInternal> {
((node, variables) => parseValue(valueFromASTUntyped(node, variables)));
this.coerceInputLiteral = config.coerceInputLiteral;
this.valueToLiteral = config.valueToLiteral;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];

Expand Down Expand Up @@ -725,7 +725,7 @@ interface GraphQLScalarTypeNormalizedConfig<TInternal, TExternal>
* you may find them useful.
*/
export interface GraphQLObjectTypeExtensions<_TSource = any, _TContext = any> {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand Down Expand Up @@ -783,7 +783,7 @@ export class GraphQLObjectType<TSource = any, TContext = any> {
this.name = assertName(config.name);
this.description = config.description;
this.isTypeOf = config.isTypeOf;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];
this._fields = (defineFieldMap<TSource, TContext>).bind(
Expand Down Expand Up @@ -854,7 +854,7 @@ function defineFieldMap<TSource, TContext>(
resolve: fieldConfig.resolve,
subscribe: fieldConfig.subscribe,
deprecationReason: fieldConfig.deprecationReason,
extensions: toObjMap(fieldConfig.extensions),
extensions: toObjMapWithSymbols(fieldConfig.extensions),
astNode: fieldConfig.astNode,
};
});
Expand All @@ -869,7 +869,7 @@ export function defineArguments(
type: argConfig.type,
defaultValue: defineDefaultValue(argName, argConfig),
deprecationReason: argConfig.deprecationReason,
extensions: toObjMap(argConfig.extensions),
extensions: toObjMapWithSymbols(argConfig.extensions),
astNode: argConfig.astNode,
}));
}
Expand Down Expand Up @@ -980,7 +980,7 @@ export interface GraphQLResolveInfo {
* you may find them useful.
*/
export interface GraphQLFieldExtensions<_TSource, _TContext, _TArgs = any> {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

export interface GraphQLFieldConfig<TSource, TContext, TArgs = any> {
Expand Down Expand Up @@ -1008,7 +1008,7 @@ export type GraphQLFieldConfigArgumentMap = ObjMap<GraphQLArgumentConfig>;
* an object which can contain all the values you need.
*/
export interface GraphQLArgumentExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

export interface GraphQLArgumentConfig {
Expand Down Expand Up @@ -1085,7 +1085,7 @@ export function defineDefaultValue(
* an object which can contain all the values you need.
*/
export interface GraphQLInterfaceTypeExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand Down Expand Up @@ -1122,7 +1122,7 @@ export class GraphQLInterfaceType<TSource = any, TContext = any> {
this.name = assertName(config.name);
this.description = config.description;
this.resolveType = config.resolveType;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];
this._fields = (defineFieldMap<TSource, TContext>).bind(
Expand Down Expand Up @@ -1206,7 +1206,7 @@ interface GraphQLInterfaceTypeNormalizedConfig<TSource, TContext>
* an object which can contain all the values you need.
*/
export interface GraphQLUnionTypeExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand Down Expand Up @@ -1247,7 +1247,7 @@ export class GraphQLUnionType {
this.name = assertName(config.name);
this.description = config.description;
this.resolveType = config.resolveType;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];

Expand Down Expand Up @@ -1324,7 +1324,7 @@ interface GraphQLUnionTypeNormalizedConfig
* an object which can contain all the values you need.
*/
export interface GraphQLEnumTypeExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

function enumValuesFromConfig(values: GraphQLEnumValueConfigMap) {
Expand All @@ -1333,7 +1333,7 @@ function enumValuesFromConfig(values: GraphQLEnumValueConfigMap) {
description: valueConfig.description,
value: valueConfig.value !== undefined ? valueConfig.value : valueName,
deprecationReason: valueConfig.deprecationReason,
extensions: toObjMap(valueConfig.extensions),
extensions: toObjMapWithSymbols(valueConfig.extensions),
astNode: valueConfig.astNode,
}));
}
Expand Down Expand Up @@ -1378,7 +1378,7 @@ export class GraphQLEnumType /* <T> */ {
constructor(config: Readonly<GraphQLEnumTypeConfig /* <T> */>) {
this.name = assertName(config.name);
this.description = config.description;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];

Expand Down Expand Up @@ -1559,7 +1559,7 @@ export type GraphQLEnumValueConfigMap /* <T> */ =
* an object which can contain all the values you need.
*/
export interface GraphQLEnumValueExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

export interface GraphQLEnumValueConfig {
Expand Down Expand Up @@ -1589,7 +1589,7 @@ export interface GraphQLEnumValue {
* an object which can contain all the values you need.
*/
export interface GraphQLInputObjectTypeExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand Down Expand Up @@ -1626,7 +1626,7 @@ export class GraphQLInputObjectType {
constructor(config: Readonly<GraphQLInputObjectTypeConfig>) {
this.name = assertName(config.name);
this.description = config.description;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];
this.isOneOf = config.isOneOf ?? false;
Expand Down Expand Up @@ -1686,7 +1686,7 @@ function defineInputFieldMap(
type: fieldConfig.type,
defaultValue: defineDefaultValue(fieldName, fieldConfig),
deprecationReason: fieldConfig.deprecationReason,
extensions: toObjMap(fieldConfig.extensions),
extensions: toObjMapWithSymbols(fieldConfig.extensions),
astNode: fieldConfig.astNode,
}));
}
Expand Down Expand Up @@ -1718,7 +1718,7 @@ interface GraphQLInputObjectTypeNormalizedConfig
* an object which can contain all the values you need.
*/
export interface GraphQLInputFieldExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

export interface GraphQLInputFieldConfig {
Expand Down
6 changes: 3 additions & 3 deletions src/type/directives.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { inspect } from '../jsutils/inspect.js';
import { instanceOf } from '../jsutils/instanceOf.js';
import type { Maybe } from '../jsutils/Maybe.js';
import { toObjMap } from '../jsutils/toObjMap.js';
import { toObjMapWithSymbols } from '../jsutils/toObjMap.js';

import type { DirectiveDefinitionNode } from '../language/ast.js';
import { DirectiveLocation } from '../language/directiveLocation.js';
Expand Down Expand Up @@ -44,7 +44,7 @@ export function assertDirective(directive: unknown): GraphQLDirective {
* an object which can contain all the values you need.
*/
export interface GraphQLDirectiveExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand All @@ -65,7 +65,7 @@ export class GraphQLDirective {
this.description = config.description;
this.locations = config.locations;
this.isRepeatable = config.isRepeatable ?? false;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;

const args = config.args ?? {};
Expand Down
6 changes: 3 additions & 3 deletions src/type/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { inspect } from '../jsutils/inspect.js';
import { instanceOf } from '../jsutils/instanceOf.js';
import type { Maybe } from '../jsutils/Maybe.js';
import type { ObjMap } from '../jsutils/ObjMap.js';
import { toObjMap } from '../jsutils/toObjMap.js';
import { toObjMapWithSymbols } from '../jsutils/toObjMap.js';

import type { GraphQLError } from '../error/GraphQLError.js';

Expand Down Expand Up @@ -61,7 +61,7 @@ export function assertSchema(schema: unknown): GraphQLSchema {
* an object which can contain all the values you need.
*/
export interface GraphQLSchemaExtensions {
[attributeName: string]: unknown;
[attributeName: string | symbol]: unknown;
}

/**
Expand Down Expand Up @@ -162,7 +162,7 @@ export class GraphQLSchema {
this.__validationErrors = config.assumeValid === true ? [] : undefined;

this.description = config.description;
this.extensions = toObjMap(config.extensions);
this.extensions = toObjMapWithSymbols(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];

Expand Down

0 comments on commit 8d99fa4

Please sign in to comment.