From 06adebe59741aa842cd508a1867222f4a40692e7 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Thu, 19 Sep 2024 12:45:26 +0100 Subject: [PATCH 1/6] Add getter for `loc` property on documents --- src/api.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/api.ts b/src/api.ts index 8b80c372..f1a79424 100644 --- a/src/api.ts +++ b/src/api.ts @@ -1,4 +1,4 @@ -import type { DocumentNode, DefinitionNode } from '@0no-co/graphql.web'; +import type { DocumentNode, DefinitionNode, Location } from '@0no-co/graphql.web'; import { Kind, parse as _parse } from '@0no-co/graphql.web'; import type { @@ -334,7 +334,21 @@ export function initGraphQLTada(): init ); } - return { kind: Kind.DOCUMENT, definitions }; + return { + kind: Kind.DOCUMENT, + definitions, + get loc(): Location { + return { + start: 0, + end: input.length, + source: { + body: input, + name: 'GraphQLTada', + locationOffset: { line: 1, column: 1 }, + }, + }; + }, + } satisfies DocumentNode; } graphql.scalar = function scalar(_schema: Schema, value: any) { From d1281a54fcfc21ea574590bec5b2f906d2df3585 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Thu, 19 Sep 2024 12:47:02 +0100 Subject: [PATCH 2/6] Remove `loc` when output isn't a fragment --- src/api.ts | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/api.ts b/src/api.ts index f1a79424..c59378c6 100644 --- a/src/api.ts +++ b/src/api.ts @@ -328,7 +328,11 @@ export function initGraphQLTada(): init } } - if (definitions[0].kind === Kind.FRAGMENT_DEFINITION && definitions[0].directives) { + let isFragment: boolean; + if ( + (isFragment = definitions[0].kind === Kind.FRAGMENT_DEFINITION) && + definitions[0].directives + ) { definitions[0].directives = definitions[0].directives.filter( (directive) => directive.name.value !== '_unmask' ); @@ -338,15 +342,17 @@ export function initGraphQLTada(): init kind: Kind.DOCUMENT, definitions, get loc(): Location { - return { - start: 0, - end: input.length, - source: { - body: input, - name: 'GraphQLTada', - locationOffset: { line: 1, column: 1 }, - }, - }; + return isFragment + ? { + start: 0, + end: input.length, + source: { + body: input, + name: 'GraphQLTada', + locationOffset: { line: 1, column: 1 }, + }, + } + : undefined; }, } satisfies DocumentNode; } From 05cacbc163716e88998e8e78bb0d548aa17472f4 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Thu, 19 Sep 2024 12:48:48 +0100 Subject: [PATCH 3/6] Add changeset --- .changeset/honest-donkeys-pay.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/honest-donkeys-pay.md diff --git a/.changeset/honest-donkeys-pay.md b/.changeset/honest-donkeys-pay.md new file mode 100644 index 00000000..9310ea4d --- /dev/null +++ b/.changeset/honest-donkeys-pay.md @@ -0,0 +1,5 @@ +--- +'gql.tada': patch +--- + +Add `loc` getter to parsed `DocumentNode` fragment outputs to ensure that using fragments created by `gql.tada`'s `graphql()` function with `graphql-tag` doesn't crash. `graphql-tag` does not treat the `DocumentNode.loc` property as optional on interpolations, which leads to intercompatibility issues. From 97336e50371065532860e269c21c2e1ed5bec98d Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Thu, 19 Sep 2024 13:16:47 +0100 Subject: [PATCH 4/6] Deduplicate loc sources recursively --- src/api.ts | 26 +++++++++++++++----------- src/utils.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/api.ts b/src/api.ts index c59378c6..5fc47a8d 100644 --- a/src/api.ts +++ b/src/api.ts @@ -22,6 +22,7 @@ import type { getDocumentType } from './selection'; import type { parseDocument, DocumentNodeLike } from './parser'; import type { getVariablesType, getScalarType } from './variables'; import type { obj, matchOr, writable, DocumentDecoration } from './utils'; +import { concatLocSources } from './utils'; /** Abstract configuration type input for your schema and scalars. * @@ -341,18 +342,21 @@ export function initGraphQLTada(): init return { kind: Kind.DOCUMENT, definitions, + // NOTE: This is only meant for `graphql-tag` compatibility and shouldn't be used for + // any other cases, since it simply appends all documents get loc(): Location { - return isFragment - ? { - start: 0, - end: input.length, - source: { - body: input, - name: 'GraphQLTada', - locationOffset: { line: 1, column: 1 }, - }, - } - : undefined; + if (isFragment) { + const body = input + concatLocSources(fragments || []); + return { + start: 0, + end: body.length, + source: { + body: body, + name: 'GraphQLTada', + locationOffset: { line: 1, column: 1 }, + }, + }; + } }, } satisfies DocumentNode; } diff --git a/src/utils.ts b/src/utils.ts index 72d24b86..65131300 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,3 +1,5 @@ +import type { DocumentNode, Location } from '@0no-co/graphql.web'; + /** Returns `T` if it matches `Constraint` without being equal to it. Failing this evaluates to `Fallback` otherwise. */ export type matchOr = Constraint extends T ? Fallback @@ -50,3 +52,29 @@ export interface DocumentDecoration { */ __ensureTypesOfVariablesAndResultMatching?: (variables: Variables) => Result; } + +let CONCAT_LOC_DEPTH = 0; +const CONCAT_LOC_SEEN = new Set(); + +interface LocationNode { + loc?: Location; +} + +/** Concatenates all fragments' `loc.source.body`s */ +export function concatLocSources(fragments: readonly LocationNode[]): string { + try { + let result = ''; + for (const fragment of fragments) { + if (!CONCAT_LOC_SEEN.has(fragment)) { + CONCAT_LOC_SEEN.add(fragment); + const { loc } = fragment; + if (loc) result += loc.source.body; + } + } + return result; + } finally { + if (--CONCAT_LOC_DEPTH === 0) { + CONCAT_LOC_SEEN.clear(); + } + } +} From 8dff8fe57fff9090b6e11cf2b84d4e2c8962f825 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Thu, 19 Sep 2024 13:17:58 +0100 Subject: [PATCH 5/6] Add tests and update note --- src/__tests__/utils.test.ts | 78 +++++++++++++++++++++++++++++++++++++ src/api.ts | 5 ++- 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/utils.test.ts diff --git a/src/__tests__/utils.test.ts b/src/__tests__/utils.test.ts new file mode 100644 index 00000000..a504e626 --- /dev/null +++ b/src/__tests__/utils.test.ts @@ -0,0 +1,78 @@ +import type { Location } from '@0no-co/graphql.web'; +import { describe, it, expect } from 'vitest'; +import { concatLocSources } from '../utils'; + +const makeLocation = (input: string): Location => ({ + start: 0, + end: input.length, + source: { + body: input, + name: 'GraphQLTada', + locationOffset: { line: 1, column: 1 }, + }, +}); + +describe('concatLocSources', () => { + it('outputs the fragments concatenated to one another', () => { + const actual = concatLocSources([{ loc: makeLocation('a') }, { loc: makeLocation('b') }]); + expect(actual).toBe('ab'); + }); + + it('works when called recursively', () => { + // NOTE: Should work repeatedly + for (let i = 0; i < 2; i++) { + const actual = concatLocSources([ + { + get loc() { + return makeLocation( + concatLocSources([{ loc: makeLocation('a') }, { loc: makeLocation('b') }]) + ); + }, + }, + { + get loc() { + return makeLocation( + concatLocSources([{ loc: makeLocation('c') }, { loc: makeLocation('d') }]) + ); + }, + }, + ]); + expect(actual).toBe('abcd'); + } + }); + + it('deduplicates recursively', () => { + // NOTE: Should work repeatedly + for (let i = 0; i < 2; i++) { + const a = { loc: makeLocation('a') }; + const b = { loc: makeLocation('b') }; + const c = { loc: makeLocation('c') }; + const d = { loc: makeLocation('d') }; + + const actual = concatLocSources([ + { + get loc() { + return makeLocation(concatLocSources([a, b, c, d])); + }, + }, + { + get loc() { + return makeLocation( + concatLocSources([ + a, + b, + c, + { + get loc() { + return makeLocation(concatLocSources([a, b, c, d])); + }, + }, + ]) + ); + }, + }, + ]); + expect(actual).toBe('abcd'); + } + }); +}); diff --git a/src/api.ts b/src/api.ts index 5fc47a8d..a159b2fe 100644 --- a/src/api.ts +++ b/src/api.ts @@ -342,9 +342,10 @@ export function initGraphQLTada(): init return { kind: Kind.DOCUMENT, definitions, - // NOTE: This is only meant for `graphql-tag` compatibility and shouldn't be used for - // any other cases, since it simply appends all documents get loc(): Location { + // NOTE: This is only meant for graphql-tag compatibility. When fragment documents + // are interpolated into other documents, graphql-tag blindly reads `document.loc` + // without checking whether it's `undefined`. if (isFragment) { const body = input + concatLocSources(fragments || []); return { From 8a29714bc2a843ff6d1c95724e64d97ed56c0153 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Thu, 19 Sep 2024 13:26:41 +0100 Subject: [PATCH 6/6] Update test and add missing increment --- src/__tests__/utils.test.ts | 29 ++++++++++++++++++++++++++++- src/utils.ts | 1 + 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/__tests__/utils.test.ts b/src/__tests__/utils.test.ts index a504e626..096b704c 100644 --- a/src/__tests__/utils.test.ts +++ b/src/__tests__/utils.test.ts @@ -49,7 +49,7 @@ describe('concatLocSources', () => { const c = { loc: makeLocation('c') }; const d = { loc: makeLocation('d') }; - const actual = concatLocSources([ + let actual = concatLocSources([ { get loc() { return makeLocation(concatLocSources([a, b, c, d])); @@ -72,6 +72,33 @@ describe('concatLocSources', () => { }, }, ]); + + expect(actual).toBe('abcd'); + + actual = concatLocSources([ + { + get loc() { + return makeLocation( + concatLocSources([ + a, + b, + c, + { + get loc() { + return makeLocation(concatLocSources([a, b, c, d])); + }, + }, + ]) + ); + }, + }, + { + get loc() { + return makeLocation(concatLocSources([a, b, c, d])); + }, + }, + ]); + expect(actual).toBe('abcd'); } }); diff --git a/src/utils.ts b/src/utils.ts index 65131300..b39d25a7 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -63,6 +63,7 @@ interface LocationNode { /** Concatenates all fragments' `loc.source.body`s */ export function concatLocSources(fragments: readonly LocationNode[]): string { try { + CONCAT_LOC_DEPTH++; let result = ''; for (const fragment of fragments) { if (!CONCAT_LOC_SEEN.has(fragment)) {