From 30179ed9a088a9a863150c085b100daf0a09413b Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 14:23:09 +0000 Subject: [PATCH 01/14] Refactor for library mode usage --- src/cli.ts | 221 ++--------------------------------------------- src/index.ts | 239 +++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 240 insertions(+), 220 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 7d98e3e..0013319 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,25 +1,9 @@ import { readFile, writeFile } from "node:fs/promises"; import { parseArgs } from "node:util"; -import { - buildSchema, - GraphQLFieldConfig, - GraphQLFieldConfigMap, - GraphQLInterfaceType, - GraphQLList, - GraphQLNamedType, - GraphQLNonNull, - GraphQLObjectType, - GraphQLOutputType, - GraphQLSchema, - GraphQLSemanticNonNull, - GraphQLType, - GraphQLUnionType, - Kind, - printSchema, - validateSchema, -} from "graphql"; -import type { Maybe } from "graphql/jsutils/Maybe"; +import { buildSchema, printSchema,validateSchema } from "graphql"; + +import { semanticToNullable,semanticToStrict } from "./index.js"; export async function main(toStrict = false) { const { @@ -51,203 +35,10 @@ export async function main(toStrict = false) { throw new Error("Invalid schema"); } - const derivedSchema = convertSchema(schema, toStrict); + const derivedSchema = toStrict + ? semanticToStrict(schema) + : semanticToNullable(schema); const newSdl = printSchema(derivedSchema); await writeFile(output, newSdl + "\n"); } - -function convertSchema(schema: GraphQLSchema, toStrict: boolean) { - const config = schema.toConfig(); - const convertType = makeConvertType(toStrict); - const derivedSchema = new GraphQLSchema({ - ...config, - query: convertType(config.query), - mutation: convertType(config.mutation), - subscription: convertType(config.subscription), - types: config.types - .filter((t) => !t.name.startsWith("__")) - .map((t) => convertType(t)), - directives: config.directives.filter((d) => d.name !== "semanticNonNull"), - }); - return derivedSchema; -} - -export function semanticToNullable(schema: GraphQLSchema) { - return convertSchema(schema, false); -} - -export function semanticToStrict(schema: GraphQLSchema) { - return convertSchema(schema, true); -} - -function makeConvertType(toStrict: boolean) { - const cache = new Map(); - - function convertFields(fields: GraphQLFieldConfigMap) { - return () => { - return Object.fromEntries( - Object.entries(fields).map(([fieldName, inSpec]) => { - const spec = applySemanticNonNullDirectiveToFieldConfig(inSpec); - return [ - fieldName, - { - ...spec, - type: convertType(spec.type), - }, - ]; - }), - ) as GraphQLFieldConfigMap; - }; - } - - function convertTypes( - types: readonly GraphQLInterfaceType[] | null | undefined, - ): undefined | (() => readonly GraphQLInterfaceType[]); - function convertTypes( - types: readonly GraphQLObjectType[], - ): () => readonly GraphQLObjectType[]; - function convertTypes( - types: readonly GraphQLNamedType[], - ): undefined | (() => readonly GraphQLNamedType[]); - function convertTypes( - types: readonly GraphQLNamedType[] | undefined, - ): undefined | (() => readonly GraphQLNamedType[]); - function convertTypes( - types: readonly GraphQLNamedType[] | null | undefined, - ): undefined | (() => readonly GraphQLNamedType[]) { - if (!types) { - return undefined; - } - return () => types.map((t) => convertType(t)); - } - - function convertType(type: null | undefined): null | undefined; - function convertType(type: GraphQLObjectType): GraphQLObjectType; - function convertType( - type: Maybe, - ): Maybe; - function convertType(type: GraphQLNamedType): GraphQLNamedType; - function convertType(type: GraphQLType): GraphQLType; - function convertType(type: GraphQLType | null | undefined) { - if (!type) { - return type; - } - if (type instanceof GraphQLSemanticNonNull) { - const unwrapped = convertType(type.ofType); - // Here's where we do our thing! - if (toStrict) { - return new GraphQLNonNull(unwrapped); - } else { - return unwrapped; - } - } else if (type instanceof GraphQLNonNull) { - return new GraphQLNonNull(convertType(type.ofType)); - } else if (type instanceof GraphQLList) { - return new GraphQLList(convertType(type.ofType)); - } - if (type.name.startsWith("__")) { - return null; - } - if (cache.has(type.name)) { - return cache.get(type.name); - } - const newType = (() => { - if (type instanceof GraphQLObjectType) { - const config = type.toConfig(); - return new GraphQLObjectType({ - ...config, - fields: convertFields(config.fields), - interfaces: convertTypes(config.interfaces), - }); - } else if (type instanceof GraphQLInterfaceType) { - const config = type.toConfig(); - return new GraphQLInterfaceType({ - ...config, - fields: convertFields(config.fields), - interfaces: convertTypes(config.interfaces), - }); - } else if (type instanceof GraphQLUnionType) { - const config = type.toConfig(); - return new GraphQLUnionType({ - ...config, - types: convertTypes(config.types), - }); - } else { - return type; - } - })(); - cache.set(type.name, newType); - return newType; - } - - return convertType; -} - -/** - * Takes a GraphQL field config and checks to see if the `@semanticNonNull` - * directive was applied; if so, converts to a field config using explicit - * GraphQLSemanticNonNull wrapper types instead. - * - * @see {@url https://www.apollographql.com/docs/kotlin/advanced/nullability/#semanticnonnull} - */ -export function applySemanticNonNullDirectiveToFieldConfig( - spec: GraphQLFieldConfig, -): GraphQLFieldConfig { - const directive = spec.astNode?.directives?.find( - (d) => d.name.value === "semanticNonNull", - ); - if (!directive) { - return spec; - } - const levelsArg = directive.arguments?.find((a) => a.name.value === "levels"); - const levels = - levelsArg?.value?.kind === Kind.LIST - ? levelsArg.value.values - .filter((v) => v.kind === Kind.INT) - .map((v) => Number(v.value)) - : [0]; - function recurse(type: GraphQLOutputType, level: number): GraphQLOutputType { - if (type instanceof GraphQLSemanticNonNull) { - // Strip semantic-non-null types; this should never happen but if someone - // uses both semantic-non-null and the `@semanticNonNull` directive, we - // want the directive to win (I guess?) - return recurse(type.ofType, level); - } else if (type instanceof GraphQLNonNull) { - const inner = recurse(type.ofType, level); - if (levels.includes(level)) { - // Semantic non-null from `inner` replaces our GrpahQLNonNull wrapper - return inner; - } else { - // Keep non-null wrapper; no semantic-non-null was added to `inner` - return new GraphQLNonNull(inner); - } - } else if (type instanceof GraphQLList) { - const inner = new GraphQLList(recurse(type.ofType, level + 1)); - if (levels.includes(level)) { - return new GraphQLSemanticNonNull(inner); - } else { - return inner; - } - } else { - if (levels.includes(level)) { - return new GraphQLSemanticNonNull(type); - } else { - return type; - } - } - } - - return { - ...spec, - type: recurse(spec.type, 0), - astNode: spec.astNode - ? { - ...spec.astNode, - directives: spec.astNode.directives?.filter( - (d) => d.name.value !== "semanticNonNull", - ), - } - : undefined, - }; -} diff --git a/src/index.ts b/src/index.ts index cf56256..c969bd3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,234 @@ -export { - applySemanticNonNullDirectiveToFieldConfig, - semanticToNullable, - semanticToStrict, -} from "./cli.js"; +import { + GraphQLFieldConfig, + GraphQLFieldConfigMap, + GraphQLInterfaceType, + GraphQLList, + GraphQLNamedType, + GraphQLNonNull, + GraphQLNullableType, + GraphQLObjectType, + GraphQLOutputType, + GraphQLSchema, + GraphQLType, + GraphQLUnionType, + Kind, +} from "graphql"; +import * as graphql from "graphql"; + +type Maybe = null | undefined | T; + +// If GraphQL doesn't have this helper function, then it doesn't natively support GraphQLSemanticNonNull +const isSemanticNonNullType = graphql.isSemanticNonNullType ?? (() => false); + +function convertSchema(schema: GraphQLSchema, toStrict: boolean) { + const config = schema.toConfig(); + const convertType = makeConvertType(toStrict); + const derivedSchema = new GraphQLSchema({ + ...config, + query: convertType(config.query), + mutation: convertType(config.mutation), + subscription: convertType(config.subscription), + types: config.types + .filter((t) => !t.name.startsWith("__")) + .map((t) => convertType(t)), + directives: config.directives.filter((d) => d.name !== "semanticNonNull"), + }); + return derivedSchema; +} + +export function semanticToNullable(schema: GraphQLSchema) { + return convertSchema(schema, false); +} + +export function semanticToStrict(schema: GraphQLSchema) { + return convertSchema(schema, true); +} + +function makeConvertType(toStrict: boolean) { + const cache = new Map(); + + function convertFields(fields: GraphQLFieldConfigMap) { + return () => { + return Object.fromEntries( + Object.entries(fields).map(([fieldName, inSpec]) => { + const spec = applySemanticNonNullDirectiveToFieldConfig( + inSpec, + toStrict, + ); + return [ + fieldName, + { + ...spec, + type: convertType(spec.type), + }, + ]; + }), + ) as GraphQLFieldConfigMap; + }; + } + + function convertTypes( + types: readonly GraphQLInterfaceType[] | null | undefined, + ): undefined | (() => readonly GraphQLInterfaceType[]); + function convertTypes( + types: readonly GraphQLObjectType[], + ): () => readonly GraphQLObjectType[]; + function convertTypes( + types: readonly GraphQLNamedType[], + ): undefined | (() => readonly GraphQLNamedType[]); + function convertTypes( + types: readonly GraphQLNamedType[] | undefined, + ): undefined | (() => readonly GraphQLNamedType[]); + function convertTypes( + types: readonly GraphQLNamedType[] | null | undefined, + ): undefined | (() => readonly GraphQLNamedType[]) { + if (!types) { + return undefined; + } + return () => types.map((t) => convertType(t)); + } + + function convertType(type: null | undefined): null | undefined; + function convertType(type: GraphQLObjectType): GraphQLObjectType; + function convertType( + type: Maybe, + ): Maybe; + function convertType(type: GraphQLNamedType): GraphQLNamedType; + function convertType(type: GraphQLType): GraphQLType; + function convertType(type: GraphQLType | null | undefined) { + if (!type) { + return type; + } + if (isSemanticNonNullType(type)) { + const unwrapped = convertType(type.ofType as GraphQLNullableType); + // Here's where we do our thing! + if (toStrict) { + return new GraphQLNonNull(unwrapped); + } else { + return unwrapped; + } + } else if (type instanceof GraphQLNonNull) { + return new GraphQLNonNull( + convertType(type.ofType as GraphQLNullableType), + ); + } else if (type instanceof GraphQLList) { + return new GraphQLList(convertType(type.ofType as GraphQLType)); + } + if (type.name.startsWith("__")) { + return null; + } + if (cache.has(type.name)) { + return cache.get(type.name); + } + const newType = (() => { + if (type instanceof GraphQLObjectType) { + const config = type.toConfig(); + return new GraphQLObjectType({ + ...config, + fields: convertFields(config.fields), + interfaces: convertTypes(config.interfaces), + }); + } else if (type instanceof GraphQLInterfaceType) { + const config = type.toConfig(); + return new GraphQLInterfaceType({ + ...config, + fields: convertFields(config.fields), + interfaces: convertTypes(config.interfaces), + }); + } else if (type instanceof GraphQLUnionType) { + const config = type.toConfig(); + return new GraphQLUnionType({ + ...config, + types: convertTypes(config.types), + }); + } else { + return type; + } + })(); + cache.set(type.name, newType); + return newType; + } + + return convertType; +} + +/** + * Takes a GraphQL field config and checks to see if the `@semanticNonNull` + * directive was applied; if so, converts to a field config that adds + * GraphQLNonNull wrapper types in the relevant places if toStrict is true. + * + * @see {@url https://www.apollographql.com/docs/kotlin/advanced/nullability/#semanticnonnull} + */ +export function applySemanticNonNullDirectiveToFieldConfig( + spec: GraphQLFieldConfig, + toStrict: boolean, +): GraphQLFieldConfig { + const directive = spec.astNode?.directives?.find( + (d) => d.name.value === "semanticNonNull", + ); + if (!directive) { + return spec; + } + + /** The AST node with the semanticNonNull directive removed */ + const filteredAstNode = { + ...spec.astNode!, + directives: spec.astNode!.directives!.filter( + (d) => d.name.value !== "semanticNonNull", + ), + }; + + if (!toStrict) { + // If we're not converting to strict, we can simply strip this directive + return { + ...spec, + astNode: filteredAstNode, + }; + } + + // Otherwise, convert semantic non-null positions to strict non-null + + const levelsArg = directive.arguments?.find((a) => a.name.value === "levels"); + const levels = + levelsArg?.value?.kind === Kind.LIST + ? levelsArg.value.values + .filter((v) => v.kind === Kind.INT) + .map((v) => Number(v.value)) + : [0]; + function recurse(type: GraphQLOutputType, level: number): GraphQLOutputType { + if (isSemanticNonNullType(type)) { + // Strip semantic-non-null types; this should never happen but if someone + // uses both semantic-non-null and the `@semanticNonNull` directive, we + // want the directive to win (I guess?) + return recurse(type.ofType, level); + } else if (type instanceof GraphQLNonNull) { + const inner = recurse(type.ofType, level); + if (levels.includes(level)) { + // Semantic non-null from `inner` replaces our GraphQLNonNull wrapper + return inner; + } else { + // Keep non-null wrapper; no semantic-non-null was added to `inner` + return new GraphQLNonNull(inner); + } + } else if (type instanceof GraphQLList) { + const inner = new GraphQLList(recurse(type.ofType, level + 1)); + if (levels.includes(level)) { + return new GraphQLNonNull(inner); + } else { + return inner; + } + } else { + if (levels.includes(level)) { + return new GraphQLNonNull(type); + } else { + return type; + } + } + } + + return { + ...spec, + type: recurse(spec.type, 0), + astNode: filteredAstNode, + }; +} From 9669fe97b2038b61536c166696534f5fa52ec37e Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 14:31:50 +0000 Subject: [PATCH 02/14] Rename function --- src/index.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index c969bd3..2df3584 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,6 +11,11 @@ import { GraphQLSchema, GraphQLType, GraphQLUnionType, + isInterfaceType, + isListType, + isNonNullType, + isObjectType, + isUnionType, Kind, } from "graphql"; import * as graphql from "graphql"; @@ -51,10 +56,7 @@ function makeConvertType(toStrict: boolean) { return () => { return Object.fromEntries( Object.entries(fields).map(([fieldName, inSpec]) => { - const spec = applySemanticNonNullDirectiveToFieldConfig( - inSpec, - toStrict, - ); + const spec = convertFieldConfig(inSpec, toStrict); return [ fieldName, { @@ -155,11 +157,11 @@ function makeConvertType(toStrict: boolean) { /** * Takes a GraphQL field config and checks to see if the `@semanticNonNull` * directive was applied; if so, converts to a field config that adds - * GraphQLNonNull wrapper types in the relevant places if toStrict is true. + * GraphQLNonNull wrapper types in the relevant places if `toStrict` is true. * * @see {@url https://www.apollographql.com/docs/kotlin/advanced/nullability/#semanticnonnull} */ -export function applySemanticNonNullDirectiveToFieldConfig( +export function convertFieldConfig( spec: GraphQLFieldConfig, toStrict: boolean, ): GraphQLFieldConfig { From a506bfe7a9d07f5f96666e8c9d9a278532fd7f0f Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 14:32:15 +0000 Subject: [PATCH 03/14] Use type guards rather than instanceof --- src/index.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/index.ts b/src/index.ts index 2df3584..6a26e6c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -109,11 +109,11 @@ function makeConvertType(toStrict: boolean) { } else { return unwrapped; } - } else if (type instanceof GraphQLNonNull) { + } else if (isNonNullType(type)) { return new GraphQLNonNull( convertType(type.ofType as GraphQLNullableType), ); - } else if (type instanceof GraphQLList) { + } else if (isListType(type)) { return new GraphQLList(convertType(type.ofType as GraphQLType)); } if (type.name.startsWith("__")) { @@ -123,21 +123,21 @@ function makeConvertType(toStrict: boolean) { return cache.get(type.name); } const newType = (() => { - if (type instanceof GraphQLObjectType) { + if (isObjectType(type)) { const config = type.toConfig(); return new GraphQLObjectType({ ...config, fields: convertFields(config.fields), interfaces: convertTypes(config.interfaces), }); - } else if (type instanceof GraphQLInterfaceType) { + } else if (isInterfaceType(type)) { const config = type.toConfig(); return new GraphQLInterfaceType({ ...config, fields: convertFields(config.fields), interfaces: convertTypes(config.interfaces), }); - } else if (type instanceof GraphQLUnionType) { + } else if (isUnionType(type)) { const config = type.toConfig(); return new GraphQLUnionType({ ...config, @@ -203,7 +203,7 @@ export function convertFieldConfig( // uses both semantic-non-null and the `@semanticNonNull` directive, we // want the directive to win (I guess?) return recurse(type.ofType, level); - } else if (type instanceof GraphQLNonNull) { + } else if (isNonNullType(type)) { const inner = recurse(type.ofType, level); if (levels.includes(level)) { // Semantic non-null from `inner` replaces our GraphQLNonNull wrapper @@ -212,7 +212,7 @@ export function convertFieldConfig( // Keep non-null wrapper; no semantic-non-null was added to `inner` return new GraphQLNonNull(inner); } - } else if (type instanceof GraphQLList) { + } else if (isListType(type)) { const inner = new GraphQLList(recurse(type.ofType, level + 1)); if (levels.includes(level)) { return new GraphQLNonNull(inner); From 676caa4c288da4cb321abcbc9fa4099b578d8146 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 14:33:24 +0000 Subject: [PATCH 04/14] Add to usage --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index 6275536..61d2bb9 100644 --- a/README.md +++ b/README.md @@ -187,3 +187,20 @@ import { schema as sourceSchema } from "./my-schema"; export const schema = semanticToStrict(sourceSchema); ``` + +## Advanced usage + +If you just want to convert a single `GraphQLFieldConfig` you can use the +`convertFieldConfig` method, passing the field config and `true` to convert +semantic non-null positions to strict non-nulls, or `false` if you want to +convert to nullable: + +```ts +const strictFieldConfig = convertFieldConfig(fieldConfig, true); +const nullableFieldConfig = convertFieldConfig(fieldConfig, false); +``` + +> [!NOTE] +> +> This method assumes that the fieldConfig has come from parsing an SDL string, +> and thus has an `astNode` that includes a `@semanticNonNull` directive. From ca6401d1f1ce6e1e5b9961a964a039c086b19082 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 14:36:12 +0000 Subject: [PATCH 05/14] Wider graphql version support --- package.json | 3 ++- yarn.lock | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 5bb9018..ec43a9d 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "author": "Benjie Gillam ", "license": "MIT", "dependencies": { - "graphql": "16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a" + "graphql": "15.x | 16.x | 17.x" }, "devDependencies": { "@tsconfig/recommended": "^1.0.7", @@ -58,6 +58,7 @@ "eslint-plugin-import": "^2.28.1", "eslint-plugin-simple-import-sort": "^10.0.0", "eslint_d": "^13.0.0", + "graphql-pr-4192": "npm:graphql@16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a", "prettier": "^3.3.3", "typescript": "^5.6.2" }, diff --git a/yarn.lock b/yarn.lock index 8bc0006..87d686e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -918,11 +918,16 @@ graphemer@^1.4.0: resolved "https://registry.yarnpkg.com/graphemer/-/graphemer-1.4.0.tgz#fb2f1d55e0e3a1849aeffc90c4fa0dd53a0e66c6" integrity sha512-EtKwoO6kxCL9WO5xipiHTZlSzBm7WLT627TqC/uVRd0HKmq8NXyebnNYxDoBi7wt8eTWrUrKXCOVaFq9x1kgag== -graphql@16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a: +"graphql-pr-4192@npm:graphql@16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a": version "16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a" resolved "https://registry.yarnpkg.com/graphql/-/graphql-16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a.tgz#c5bbcdb258959b98352bcd9ea7f17790647113ae" integrity sha512-P8UYoxSUI1KGr9O5f+AMA3TuLYxOcELoQebxGrnVAIUHM6HCpiLDT+CylrBWEBmvcc7S0xRFRiwvgwzChzLTyQ== +"graphql@15.x | 16.x | 17.x": + version "17.0.0-alpha.8" + resolved "https://registry.yarnpkg.com/graphql/-/graphql-17.0.0-alpha.8.tgz#dba4a0cbe3efe8243666726f1b4ffd65f87d9b06" + integrity sha512-j9Jn56NCWVaLMt1hSNkMDoCuAisBwY3bxp/5tbrJuPtNtHg9dAf4NjKnlVDCksVP3jBVcipFaEXKWsdNxTlcyg== + has-bigints@^1.0.1, has-bigints@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/has-bigints/-/has-bigints-1.0.2.tgz#0871bd3e3d51626f6ca0966668ba35d5602d6eaa" From f5dfa8c3fed7685792c0aa098e488a954fc0b5c4 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 14:43:32 +0000 Subject: [PATCH 06/14] Add test for schema only using directives --- .../schema-with-directive-only.test.graphql | 39 +++++++++++++++++++ ...chema-with-directive-only.nullable.graphql | 35 +++++++++++++++++ .../schema-with-directive-only.strict.graphql | 35 +++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 __tests__/schema-with-directive-only.test.graphql create mode 100644 __tests__/snapshots/schema-with-directive-only.nullable.graphql create mode 100644 __tests__/snapshots/schema-with-directive-only.strict.graphql diff --git a/__tests__/schema-with-directive-only.test.graphql b/__tests__/schema-with-directive-only.test.graphql new file mode 100644 index 0000000..567b544 --- /dev/null +++ b/__tests__/schema-with-directive-only.test.graphql @@ -0,0 +1,39 @@ +directive @semanticNonNull(levels: [Int!]) on FIELD_DEFINITION + +type Query { + allThings(includingArchived: Boolean, first: Int!): ThingConnection + @semanticNonNull +} + +type ThingConnection { + pageInfo: PageInfo! + nodes: [Thing] @semanticNonNull(levels: [0, 1]) +} + +type PageInfo { + startCursor: String @semanticNonNull(levels: [0]) + endCursor: String @semanticNonNull + hasNextPage: Boolean @semanticNonNull + hasPreviousPage: Boolean @semanticNonNull +} + +interface Thing { + id: ID! + name: String @semanticNonNull + description: String +} + +type Book implements Thing { + id: ID! + name: String @semanticNonNull + description: String + # Test that this non-null gets stripped + pages: Int! @semanticNonNull +} + +type Car implements Thing { + id: ID! + name: String @semanticNonNull + description: String + mileage: Float @semanticNonNull +} diff --git a/__tests__/snapshots/schema-with-directive-only.nullable.graphql b/__tests__/snapshots/schema-with-directive-only.nullable.graphql new file mode 100644 index 0000000..b1e92ee --- /dev/null +++ b/__tests__/snapshots/schema-with-directive-only.nullable.graphql @@ -0,0 +1,35 @@ +type Query { + allThings(includingArchived: Boolean, first: Int!): ThingConnection +} + +type ThingConnection { + pageInfo: PageInfo! + nodes: [Thing] +} + +type PageInfo { + startCursor: String + endCursor: String + hasNextPage: Boolean + hasPreviousPage: Boolean +} + +interface Thing { + id: ID! + name: String + description: String +} + +type Book implements Thing { + id: ID! + name: String + description: String + pages: Int +} + +type Car implements Thing { + id: ID! + name: String + description: String + mileage: Float +} diff --git a/__tests__/snapshots/schema-with-directive-only.strict.graphql b/__tests__/snapshots/schema-with-directive-only.strict.graphql new file mode 100644 index 0000000..fb17adf --- /dev/null +++ b/__tests__/snapshots/schema-with-directive-only.strict.graphql @@ -0,0 +1,35 @@ +type Query { + allThings(includingArchived: Boolean, first: Int!): ThingConnection! +} + +type ThingConnection { + pageInfo: PageInfo! + nodes: [Thing!]! +} + +type PageInfo { + startCursor: String! + endCursor: String! + hasNextPage: Boolean! + hasPreviousPage: Boolean! +} + +interface Thing { + id: ID! + name: String! + description: String +} + +type Book implements Thing { + id: ID! + name: String! + description: String + pages: Int! +} + +type Car implements Thing { + id: ID! + name: String! + description: String + mileage: Float! +} From 2d8277baefff15ee4ee929e99b236976fa587402 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 15:31:42 +0000 Subject: [PATCH 07/14] Non-null shouldn't be stripped by semantic --- __tests__/schema-with-directive-only.test.graphql | 2 +- __tests__/schema-with-directive.test.graphql | 2 +- __tests__/schema.test.graphql | 2 +- __tests__/snapshots/schema-with-directive-only.nullable.graphql | 2 +- __tests__/snapshots/schema-with-directive.nullable.graphql | 2 +- __tests__/snapshots/schema.nullable.graphql | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/__tests__/schema-with-directive-only.test.graphql b/__tests__/schema-with-directive-only.test.graphql index 567b544..424d543 100644 --- a/__tests__/schema-with-directive-only.test.graphql +++ b/__tests__/schema-with-directive-only.test.graphql @@ -27,7 +27,7 @@ type Book implements Thing { id: ID! name: String @semanticNonNull description: String - # Test that this non-null gets stripped + # Test that this non-null is retained pages: Int! @semanticNonNull } diff --git a/__tests__/schema-with-directive.test.graphql b/__tests__/schema-with-directive.test.graphql index 4835ba8..0fb67a8 100644 --- a/__tests__/schema-with-directive.test.graphql +++ b/__tests__/schema-with-directive.test.graphql @@ -28,7 +28,7 @@ type Book implements Thing { # Test that this semantic-non-null doesn't cause issues name: String* @semanticNonNull description: String - # Test that this non-null gets stripped + # Test that this non-null is retained pages: Int! @semanticNonNull } diff --git a/__tests__/schema.test.graphql b/__tests__/schema.test.graphql index 32d0562..61851dd 100644 --- a/__tests__/schema.test.graphql +++ b/__tests__/schema.test.graphql @@ -24,7 +24,7 @@ type Book implements Thing { id: ID! name: String* description: String - pages: Int* + pages: Int! } type Car implements Thing { diff --git a/__tests__/snapshots/schema-with-directive-only.nullable.graphql b/__tests__/snapshots/schema-with-directive-only.nullable.graphql index b1e92ee..21407b4 100644 --- a/__tests__/snapshots/schema-with-directive-only.nullable.graphql +++ b/__tests__/snapshots/schema-with-directive-only.nullable.graphql @@ -24,7 +24,7 @@ type Book implements Thing { id: ID! name: String description: String - pages: Int + pages: Int! } type Car implements Thing { diff --git a/__tests__/snapshots/schema-with-directive.nullable.graphql b/__tests__/snapshots/schema-with-directive.nullable.graphql index b1e92ee..21407b4 100644 --- a/__tests__/snapshots/schema-with-directive.nullable.graphql +++ b/__tests__/snapshots/schema-with-directive.nullable.graphql @@ -24,7 +24,7 @@ type Book implements Thing { id: ID! name: String description: String - pages: Int + pages: Int! } type Car implements Thing { diff --git a/__tests__/snapshots/schema.nullable.graphql b/__tests__/snapshots/schema.nullable.graphql index b1e92ee..21407b4 100644 --- a/__tests__/snapshots/schema.nullable.graphql +++ b/__tests__/snapshots/schema.nullable.graphql @@ -24,7 +24,7 @@ type Book implements Thing { id: ID! name: String description: String - pages: Int + pages: Int! } type Car implements Thing { From f39985cb97d3bf05c0067ac24009cae8f298f6cf Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 15:32:08 +0000 Subject: [PATCH 08/14] Only run tests that the current graphql version supports --- __tests__/index.test.mjs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/__tests__/index.test.mjs b/__tests__/index.test.mjs index 900551b..3070a27 100644 --- a/__tests__/index.test.mjs +++ b/__tests__/index.test.mjs @@ -1,17 +1,28 @@ // @ts-check -import { test } from "node:test"; import * as assert from "node:assert"; -import { semanticToStrict, semanticToNullable } from "../dist/index.js"; -import { buildSchema, printSchema } from "graphql"; import { readdir, readFile } from "node:fs/promises"; +import { test } from "node:test"; + +import * as graphql from "graphql"; + +import { semanticToNullable, semanticToStrict } from "../dist/index.js"; + +const { buildSchema, printSchema } = graphql; + +const isSemanticNonNullType = /** @type {any} */ (graphql) + .isSemanticNonNullType; const TEST_DIR = import.meta.dirname; const files = await readdir(TEST_DIR); +const skip = test.skip.bind(test); for (const file of files) { if (file.endsWith(".test.graphql") && !file.startsWith(".")) { - test(file.replace(/\.test\.graphql$/, ""), async () => { + const pureDirective = file === "schema-with-directive-only.test.graphql"; + const maybeTest = + pureDirective || isSemanticNonNullType != null ? test : skip; + maybeTest(file.replace(/\.test\.graphql$/, ""), async () => { const sdl = await readFile(TEST_DIR + "/" + file, "utf8"); const schema = buildSchema(sdl); await test("semantic-to-strict", async () => { From 41cac0ebd67615ecf178e432a747988e9313d925 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 15:32:31 +0000 Subject: [PATCH 09/14] Fix logic to match new tests --- src/index.ts | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 6a26e6c..40ea24d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -23,7 +23,8 @@ import * as graphql from "graphql"; type Maybe = null | undefined | T; // If GraphQL doesn't have this helper function, then it doesn't natively support GraphQLSemanticNonNull -const isSemanticNonNullType = graphql.isSemanticNonNullType ?? (() => false); +const isSemanticNonNullType = + (graphql as any).isSemanticNonNullType ?? (() => false); function convertSchema(schema: GraphQLSchema, toStrict: boolean) { const config = schema.toConfig(); @@ -102,7 +103,9 @@ function makeConvertType(toStrict: boolean) { return type; } if (isSemanticNonNullType(type)) { - const unwrapped = convertType(type.ofType as GraphQLNullableType); + const unwrapped = convertType( + (type as any).ofType as GraphQLNullableType, + ); // Here's where we do our thing! if (toStrict) { return new GraphQLNonNull(unwrapped); @@ -180,16 +183,6 @@ export function convertFieldConfig( ), }; - if (!toStrict) { - // If we're not converting to strict, we can simply strip this directive - return { - ...spec, - astNode: filteredAstNode, - }; - } - - // Otherwise, convert semantic non-null positions to strict non-null - const levelsArg = directive.arguments?.find((a) => a.name.value === "levels"); const levels = levelsArg?.value?.kind === Kind.LIST @@ -202,25 +195,27 @@ export function convertFieldConfig( // Strip semantic-non-null types; this should never happen but if someone // uses both semantic-non-null and the `@semanticNonNull` directive, we // want the directive to win (I guess?) - return recurse(type.ofType, level); + return recurse( + (type as any).ofType as GraphQLNullableType & GraphQLOutputType, + level, + ); } else if (isNonNullType(type)) { const inner = recurse(type.ofType, level); - if (levels.includes(level)) { - // Semantic non-null from `inner` replaces our GraphQLNonNull wrapper + if (isNonNullType(inner)) { return inner; } else { - // Keep non-null wrapper; no semantic-non-null was added to `inner` + // Carry the non-null through no matter what semantic says return new GraphQLNonNull(inner); } } else if (isListType(type)) { const inner = new GraphQLList(recurse(type.ofType, level + 1)); - if (levels.includes(level)) { + if (toStrict && levels.includes(level)) { return new GraphQLNonNull(inner); } else { return inner; } } else { - if (levels.includes(level)) { + if (toStrict && levels.includes(level)) { return new GraphQLNonNull(type); } else { return type; From e9d56cf3e2d4ba012994628e5f2bf50d6b034f58 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 16:13:37 +0000 Subject: [PATCH 10/14] Run tests against multiple versions of GraphQL.js --- __tests__/graphql-pr-4192.test.mjs | 3 ++ __tests__/graphql15.test.mjs | 3 ++ __tests__/graphql16.test.mjs | 3 ++ __tests__/graphql17.test.mjs | 3 ++ __tests__/index.test.mjs | 54 ------------------------ __tests__/runTest.mjs | 68 ++++++++++++++++++++++++++++++ package.json | 5 ++- yarn.lock | 15 +++++++ 8 files changed, 99 insertions(+), 55 deletions(-) create mode 100644 __tests__/graphql-pr-4192.test.mjs create mode 100644 __tests__/graphql15.test.mjs create mode 100644 __tests__/graphql16.test.mjs create mode 100644 __tests__/graphql17.test.mjs delete mode 100644 __tests__/index.test.mjs create mode 100644 __tests__/runTest.mjs diff --git a/__tests__/graphql-pr-4192.test.mjs b/__tests__/graphql-pr-4192.test.mjs new file mode 100644 index 0000000..e0f2df5 --- /dev/null +++ b/__tests__/graphql-pr-4192.test.mjs @@ -0,0 +1,3 @@ +import { runTest } from "./runTest.mjs"; + +runTest("graphql-pr-4192"); diff --git a/__tests__/graphql15.test.mjs b/__tests__/graphql15.test.mjs new file mode 100644 index 0000000..19e6e08 --- /dev/null +++ b/__tests__/graphql15.test.mjs @@ -0,0 +1,3 @@ +import { runTest } from "./runTest.mjs"; + +runTest("graphql15"); diff --git a/__tests__/graphql16.test.mjs b/__tests__/graphql16.test.mjs new file mode 100644 index 0000000..688238f --- /dev/null +++ b/__tests__/graphql16.test.mjs @@ -0,0 +1,3 @@ +import { runTest } from "./runTest.mjs"; + +runTest("graphql16"); diff --git a/__tests__/graphql17.test.mjs b/__tests__/graphql17.test.mjs new file mode 100644 index 0000000..6ff52fd --- /dev/null +++ b/__tests__/graphql17.test.mjs @@ -0,0 +1,3 @@ +import { runTest } from "./runTest.mjs"; + +runTest("graphql17"); diff --git a/__tests__/index.test.mjs b/__tests__/index.test.mjs deleted file mode 100644 index 3070a27..0000000 --- a/__tests__/index.test.mjs +++ /dev/null @@ -1,54 +0,0 @@ -// @ts-check - -import * as assert from "node:assert"; -import { readdir, readFile } from "node:fs/promises"; -import { test } from "node:test"; - -import * as graphql from "graphql"; - -import { semanticToNullable, semanticToStrict } from "../dist/index.js"; - -const { buildSchema, printSchema } = graphql; - -const isSemanticNonNullType = /** @type {any} */ (graphql) - .isSemanticNonNullType; - -const TEST_DIR = import.meta.dirname; -const files = await readdir(TEST_DIR); -const skip = test.skip.bind(test); - -for (const file of files) { - if (file.endsWith(".test.graphql") && !file.startsWith(".")) { - const pureDirective = file === "schema-with-directive-only.test.graphql"; - const maybeTest = - pureDirective || isSemanticNonNullType != null ? test : skip; - maybeTest(file.replace(/\.test\.graphql$/, ""), async () => { - const sdl = await readFile(TEST_DIR + "/" + file, "utf8"); - const schema = buildSchema(sdl); - await test("semantic-to-strict", async () => { - const expectedSdl = await readFile( - TEST_DIR + "/snapshots/" + file.replace(".test.", ".strict."), - "utf8", - ); - const converted = semanticToStrict(schema); - assert.equal( - printSchema(converted).trim(), - expectedSdl.trim(), - "Expected semantic-to-strict to match", - ); - }); - await test("semantic-to-nullable", async () => { - const expectedSdl = await readFile( - TEST_DIR + "/snapshots/" + file.replace(".test.", ".nullable."), - "utf8", - ); - const converted = semanticToNullable(schema); - assert.equal( - printSchema(converted).trim(), - expectedSdl.trim(), - "Expected semantic-to-nullable to match", - ); - }); - }); - } -} diff --git a/__tests__/runTest.mjs b/__tests__/runTest.mjs new file mode 100644 index 0000000..d0b72a0 --- /dev/null +++ b/__tests__/runTest.mjs @@ -0,0 +1,68 @@ +// @ts-check + +import * as assert from "node:assert"; +import { readdir, readFile } from "node:fs/promises"; +import { test } from "node:test"; + +const TEST_DIR = import.meta.dirname; +const files = await readdir(TEST_DIR); +const skip = test.skip.bind(test); + +/** @param graphqlModuleName {string} */ +export const runTest = async (graphqlModuleName) => { + test(graphqlModuleName, async (t) => { + const mod = await import(graphqlModuleName); + const { default: defaultExport, ...namedExports } = mod; + const mockGraphql = t.mock.module("graphql", { + cache: true, + defaultExport, + namedExports, + }); + const graphql = await import("graphql"); + const { buildSchema, printSchema } = graphql; + const isSemanticNonNullType = /** @type {any} */ (graphql) + .isSemanticNonNullType; + + const { semanticToNullable, semanticToStrict } = await import( + `../dist/index.js?graphql=${graphqlModuleName}` + ); + + for (const file of files) { + if (file.endsWith(".test.graphql") && !file.startsWith(".")) { + const pureDirective = + file === "schema-with-directive-only.test.graphql"; + const maybeTest = + pureDirective || isSemanticNonNullType != null ? test : skip; + await maybeTest(file.replace(/\.test\.graphql$/, ""), async () => { + const sdl = await readFile(TEST_DIR + "/" + file, "utf8"); + const schema = buildSchema(sdl); + await test("semantic-to-strict", async () => { + const expectedSdl = await readFile( + TEST_DIR + "/snapshots/" + file.replace(".test.", ".strict."), + "utf8", + ); + const converted = semanticToStrict(schema); + assert.equal( + printSchema(converted).trim(), + expectedSdl.trim(), + "Expected semantic-to-strict to match", + ); + }); + await test("semantic-to-nullable", async () => { + const expectedSdl = await readFile( + TEST_DIR + "/snapshots/" + file.replace(".test.", ".nullable."), + "utf8", + ); + const converted = semanticToNullable(schema); + assert.equal( + printSchema(converted).trim(), + expectedSdl.trim(), + "Expected semantic-to-nullable to match", + ); + }); + }); + } + } + mockGraphql.restore(); + }); +}; diff --git a/package.json b/package.json index ec43a9d..09add16 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ }, "scripts": { "prepack": "tsc && chmod +x dist/cli/*.js", - "test": "node --test", + "test": "node --test --experimental-test-module-mocks", "watch": "tsc --watch", "lint": "yarn prettier:check && eslint --ext .js,.jsx,.ts,.tsx,.graphql .", "lint:fix": "eslint --ext .js,.jsx,.ts,.tsx,.graphql . --fix; prettier --cache --ignore-path .eslintignore --write '**/*.{js,jsx,ts,tsx,graphql,md,json}'", @@ -59,6 +59,9 @@ "eslint-plugin-simple-import-sort": "^10.0.0", "eslint_d": "^13.0.0", "graphql-pr-4192": "npm:graphql@16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a", + "graphql15": "npm:graphql@15.x", + "graphql16": "npm:graphql@16.x", + "graphql17": "npm:graphql@17.x", "prettier": "^3.3.3", "typescript": "^5.6.2" }, diff --git a/yarn.lock b/yarn.lock index 87d686e..d63fa90 100644 --- a/yarn.lock +++ b/yarn.lock @@ -923,6 +923,21 @@ graphemer@^1.4.0: resolved "https://registry.yarnpkg.com/graphql/-/graphql-16.9.0-canary.pr.4192.1813397076f44a55e5798478e7321db9877de97a.tgz#c5bbcdb258959b98352bcd9ea7f17790647113ae" integrity sha512-P8UYoxSUI1KGr9O5f+AMA3TuLYxOcELoQebxGrnVAIUHM6HCpiLDT+CylrBWEBmvcc7S0xRFRiwvgwzChzLTyQ== +"graphql15@npm:graphql@15.x": + version "15.10.1" + resolved "https://registry.yarnpkg.com/graphql/-/graphql-15.10.1.tgz#e9ff3bb928749275477f748b14aa5c30dcad6f2f" + integrity sha512-BL/Xd/T9baO6NFzoMpiMD7YUZ62R6viR5tp/MULVEnbYJXZA//kRNW7J0j1w/wXArgL0sCxhDfK5dczSKn3+cg== + +"graphql16@npm:graphql@16.x": + version "16.10.0" + resolved "https://registry.yarnpkg.com/graphql/-/graphql-16.10.0.tgz#24c01ae0af6b11ea87bf55694429198aaa8e220c" + integrity sha512-AjqGKbDGUFRKIRCP9tCKiIGHyriz2oHEbPIbEtcSLSs4YjReZOIPQQWek4+6hjw62H9QShXHyaGivGiYVLeYFQ== + +"graphql17@npm:graphql@17.x": + version "17.0.0-alpha.8" + resolved "https://registry.yarnpkg.com/graphql/-/graphql-17.0.0-alpha.8.tgz#dba4a0cbe3efe8243666726f1b4ffd65f87d9b06" + integrity sha512-j9Jn56NCWVaLMt1hSNkMDoCuAisBwY3bxp/5tbrJuPtNtHg9dAf4NjKnlVDCksVP3jBVcipFaEXKWsdNxTlcyg== + "graphql@15.x | 16.x | 17.x": version "17.0.0-alpha.8" resolved "https://registry.yarnpkg.com/graphql/-/graphql-17.0.0-alpha.8.tgz#dba4a0cbe3efe8243666726f1b4ffd65f87d9b06" From 1de9dc424ca2c4d6eae87747ff61bdfbc4f45473 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 18 Mar 2025 16:20:02 +0000 Subject: [PATCH 11/14] Add note --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 61d2bb9..2a055bd 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,11 @@ For the directive, the two conversions work like this: | semantic-to-nullable | `[Int] @semanticNonNull(levels: [0,1])` | `[Int]` | | semantic-to-strict | `[Int] @semanticNonNull(levels: [0,1])` | `[Int!]!` | +> [!NOTE] +> +> An existing strictly non-nullable type (`Int!`) will remain unchanged whether +> or not `@semanticNonNull` applies to that level. + ### `GraphQLSemanticNonNull` wrapper type How the `GraphQLSemanticNonNull` type is represented syntactically in SDL is yet From f778e689e9af55d3fa1dac1bdb354578f8f041b5 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 19 Mar 2025 09:49:57 +0000 Subject: [PATCH 12/14] Also run CI on 22 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42ce208..fe6ee0a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: strategy: matrix: - node-version: [20.x] + node-version: [20.x, 22.x] steps: - uses: actions/checkout@v3 From 1d4622068abe5875089e8401c2194e491145af38 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 19 Mar 2025 09:59:28 +0000 Subject: [PATCH 13/14] Reduce any usage --- src/index.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/index.ts b/src/index.ts index 40ea24d..d149e2a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -23,7 +23,10 @@ import * as graphql from "graphql"; type Maybe = null | undefined | T; // If GraphQL doesn't have this helper function, then it doesn't natively support GraphQLSemanticNonNull -const isSemanticNonNullType = +const isSemanticNonNullType: ( + t: unknown, +) => t is { ofType: GraphQLNullableType } = + // eslint-disable-next-line @typescript-eslint/no-explicit-any (graphql as any).isSemanticNonNullType ?? (() => false); function convertSchema(schema: GraphQLSchema, toStrict: boolean) { @@ -103,9 +106,7 @@ function makeConvertType(toStrict: boolean) { return type; } if (isSemanticNonNullType(type)) { - const unwrapped = convertType( - (type as any).ofType as GraphQLNullableType, - ); + const unwrapped = convertType(type.ofType); // Here's where we do our thing! if (toStrict) { return new GraphQLNonNull(unwrapped); @@ -195,10 +196,7 @@ export function convertFieldConfig( // Strip semantic-non-null types; this should never happen but if someone // uses both semantic-non-null and the `@semanticNonNull` directive, we // want the directive to win (I guess?) - return recurse( - (type as any).ofType as GraphQLNullableType & GraphQLOutputType, - level, - ); + return recurse(type.ofType, level); } else if (isNonNullType(type)) { const inner = recurse(type.ofType, level); if (isNonNullType(inner)) { From 4013319e498540992cad521f31eeafbc7963d4ef Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 19 Mar 2025 09:59:35 +0000 Subject: [PATCH 14/14] Lint fixes --- src/cli.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 0013319..beeffa0 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,9 +1,9 @@ import { readFile, writeFile } from "node:fs/promises"; import { parseArgs } from "node:util"; -import { buildSchema, printSchema,validateSchema } from "graphql"; +import { buildSchema, printSchema, validateSchema } from "graphql"; -import { semanticToNullable,semanticToStrict } from "./index.js"; +import { semanticToNullable, semanticToStrict } from "./index.js"; export async function main(toStrict = false) { const {