Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduction in types accepted for type definitions #3592

Merged
merged 10 commits into from
Aug 17, 2023
6 changes: 6 additions & 0 deletions .changeset/khaki-mayflies-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@neo4j/graphql": major
"@neo4j/graphql-ogm": major
---

The Neo4j GraphQL Library now only accepts a `string`, `DocumentNode` or an array containing these types. A callback function returning these is also accepted. This is a reduction from `TypeSource` which also included types such as `GraphQLSchema` and `DefinitionNode`, which would have resulted in unexpected behaviour if passed in.
48 changes: 39 additions & 9 deletions packages/graphql/src/classes/Neo4jGraphQL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import type { Neo4jGraphQLSchemaModel } from "../schema-model/Neo4jGraphQLSchema
import { composeResolvers } from "@graphql-tools/resolvers-composition";
import type { IExecutableSchemaDefinition } from "@graphql-tools/schema";
import { addResolversToSchema, makeExecutableSchema } from "@graphql-tools/schema";
import type { TypeSource } from "@graphql-tools/utils";
import { forEachField, getResolversFromSchema } from "@graphql-tools/utils";
import type { DocumentNode, GraphQLSchema } from "graphql";
import type { Driver, SessionConfig } from "neo4j-driver";
Expand All @@ -50,8 +49,10 @@ import { makeDocumentToAugment } from "../schema/make-document-to-augment";
import { Neo4jGraphQLAuthorization } from "./authorization/Neo4jGraphQLAuthorization";
import { Neo4jGraphQLSubscriptionsDefaultEngine } from "./Neo4jGraphQLSubscriptionsDefaultEngine";

type TypeDefinitions = string | DocumentNode | TypeDefinitions[] | (() => TypeDefinitions);

export interface Neo4jGraphQLConstructor {
typeDefs: TypeSource;
typeDefs: TypeDefinitions;
resolvers?: IExecutableSchemaDefinition["resolvers"];
features?: Neo4jFeaturesSettings;
driver?: Driver;
Expand All @@ -60,7 +61,7 @@ export interface Neo4jGraphQLConstructor {
}

class Neo4jGraphQL {
private typeDefs: TypeSource;
private typeDefs: TypeDefinitions;
private resolvers?: IExecutableSchemaDefinition["resolvers"];

private driver?: Driver;
Expand Down Expand Up @@ -208,6 +209,39 @@ class Neo4jGraphQL {
return this._relationships;
}

/**
* Currently just merges all type definitions into a document. Eventual intention described below:
*
* Normalizes the user's type definitions using the method with the lowest risk of side effects:
* - Type definitions of type `string` are parsed using the `parse` function from the reference GraphQL implementation.
* - Type definitions of type `DocumentNode` are returned as they are.
* - Type definitions in arrays are merged using `mergeTypeDefs` from `@graphql-tools/merge`.
* - Callbacks are resolved to a type which can be parsed into a document.
*
* This method maps to the Type Definition Normalization stage of the Schema Generation lifecycle.
*
* @param {TypeDefinitions} typeDefinitions - The unnormalized type definitions.
* @returns {DocumentNode} The normalized type definitons as a document.
*/
private normalizeTypeDefinitions(typeDefinitions: TypeDefinitions): DocumentNode {
// TODO: The dream: minimal modification of the type definitions. However, this does not merge extensions, which we can't currently deal with in translation.
// if (typeof typeDefinitions === "function") {
// return this.normalizeTypeDefinitions(typeDefinitions());
// }

// if (typeof typeDefinitions === "string") {
// return parse(typeDefinitions);
// }

// if (Array.isArray(typeDefinitions)) {
// return mergeTypeDefs(typeDefinitions);
// }

// return typeDefinitions;

return mergeTypeDefs(typeDefinitions);
}

private addDefaultFieldResolvers(schema: GraphQLSchema): GraphQLSchema {
forEachField(schema, (field) => {
if (!field.resolve) {
Expand All @@ -228,10 +262,6 @@ class Neo4jGraphQL {
}
}

private getDocument(typeDefs: TypeSource): DocumentNode {
return mergeTypeDefs(typeDefs);
}

private async getNeo4jDatabaseInfo(driver: Driver, sessionConfig?: SessionConfig): Promise<Neo4jDatabaseInfo> {
const executorConstructorParam: ExecutorConstructorParam = {
executionContext: driver,
Expand Down Expand Up @@ -310,7 +340,7 @@ class Neo4jGraphQL {

private generateExecutableSchema(): Promise<GraphQLSchema> {
return new Promise((resolve) => {
const initialDocument = this.getDocument(this.typeDefs);
const initialDocument = this.normalizeTypeDefinitions(this.typeDefs);

if (this.validate) {
validateDocument({ document: initialDocument, features: this.features });
Expand Down Expand Up @@ -350,7 +380,7 @@ class Neo4jGraphQL {
// Import only when needed to avoid issues if GraphQL 15 being used
const { Subgraph } = await import("./Subgraph");

const initialDocument = this.getDocument(this.typeDefs);
const initialDocument = this.normalizeTypeDefinitions(this.typeDefs);
const subgraph = new Subgraph(this.typeDefs);

const { directives, types } = subgraph.getValidationDefinitions();
Expand Down
13 changes: 10 additions & 3 deletions packages/graphql/tests/e2e/federation/setup/subgraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,22 @@
* limitations under the License.
*/

import type { TypeSource } from "@graphql-tools/utils";
import type { GraphQLSchema } from "graphql";
import type { DocumentNode, GraphQLSchema } from "graphql";
import type * as neo4j from "neo4j-driver";
import { Neo4jGraphQL } from "../../../../src";

export class TestSubgraph {
library: Neo4jGraphQL;

constructor({ typeDefs, resolvers, driver }: { typeDefs: TypeSource; resolvers?: any; driver: neo4j.Driver }) {
constructor({
typeDefs,
resolvers,
driver,
}: {
typeDefs: string | DocumentNode;
resolvers?: any;
driver: neo4j.Driver;
}) {
this.library = new Neo4jGraphQL({
typeDefs,
resolvers,
Expand Down
Loading