Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/swift-ghosts-end.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@0no-co/graphqlsp': patch
---

Correctly identify missing fragments for gql.tada graphql call-expressions
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- name: Setup pnpm
uses: pnpm/action-setup@v3
with:
version: 9
version: 8.6.1
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not in line with the regular ci

run_install: false

- name: Get pnpm store directory
Expand Down
11 changes: 10 additions & 1 deletion packages/example-tada/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,16 @@ const PokemonQuery = graphql(`
}
`, [PokemonFields, Fields.Pokemon])

const persisted = graphql.persisted<typeof PokemonQuery>("sha256:78c769ed6cfef67e17e579a2abfe4da27bd51e09ed832a88393148bcce4c5a7d")
const Test = graphql(`
query Po($id: ID!) {
pokemon(id: $id) {
id
fleeRate
...Pok
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now correctly shows an error

...pokemonFields
}
}
`, [])

const Pokemons = () => {
const [result] = useQuery({
Expand Down
24 changes: 19 additions & 5 deletions packages/graphqlsp/src/ast/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function resolveIdentifierToGraphQLCall(
return checks.isGraphQLCall(value, checker) ? value : null;
}

function unrollFragment(
export function unrollFragment(
element: ts.Identifier,
info: ts.server.PluginCreateInfo,
checker: ts.TypeChecker | undefined
Expand Down Expand Up @@ -135,13 +135,16 @@ export function findAllCallExpressions(
nodes: Array<{
node: ts.StringLiteralLike;
schema: string | null;
// For gql.tada call-expressions, this contains the identifiers of explicitly declared fragments
tadaFragmentRefs?: readonly ts.Identifier[] | null;
}>;
fragments: Array<FragmentDefinitionNode>;
} {
const typeChecker = info.languageService.getProgram()?.getTypeChecker();
const result: Array<{
node: ts.StringLiteralLike;
schema: string | null;
tadaFragmentRefs?: readonly ts.Identifier[];
}> = [];
let fragments: Array<FragmentDefinitionNode> = [];
let hasTriedToFindFragments = shouldSearchFragments ? false : true;
Expand All @@ -160,18 +163,30 @@ export function findAllCallExpressions(
const name = checks.getSchemaName(node, typeChecker);
const text = node.arguments[0];
const fragmentRefs = resolveTadaFragmentArray(node.arguments[1]);
const isTadaCall = checks.isTadaGraphQLCall(node, typeChecker);

if (!hasTriedToFindFragments && !fragmentRefs) {
hasTriedToFindFragments = true;
fragments.push(...getAllFragments(sourceFile.fileName, node, info));
// Only collect global fragments if this is NOT a gql.tada call
if (!isTadaCall) {
hasTriedToFindFragments = true;
fragments.push(...getAllFragments(node, info));
}
} else if (fragmentRefs) {
for (const identifier of fragmentRefs) {
fragments.push(...unrollFragment(identifier, info, typeChecker));
}
}

if (text && ts.isStringLiteralLike(text)) {
result.push({ node: text, schema: name });
result.push({
node: text,
schema: name,
tadaFragmentRefs: isTadaCall
? fragmentRefs === undefined
? []
: fragmentRefs
: undefined,
});
}
}
find(sourceFile);
Expand Down Expand Up @@ -213,7 +228,6 @@ export function findAllPersistedCallExpressions(
}

export function getAllFragments(
fileName: string,
node: ts.Node,
info: ts.server.PluginCreateInfo
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/graphqlsp/src/autoComplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function getGraphQLCompletions(
return undefined;

const queryText = node.arguments[0].getText().slice(1, -1);
const fragments = getAllFragments(filename, node, info);
const fragments = getAllFragments(node, info);

text = `${queryText}\n${fragments.map(x => print(x)).join('\n')}`;
cursor = new Cursor(foundToken.line, foundToken.start - 1);
Expand Down
17 changes: 17 additions & 0 deletions packages/graphqlsp/src/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
findAllPersistedCallExpressions,
findAllTaggedTemplateNodes,
getSource,
unrollFragment,
} from './ast';
import { resolveTemplate } from './ast/resolve';
import { UNUSED_FIELD_CODE, checkFieldUsageInFile } from './fieldUsage';
Expand Down Expand Up @@ -344,6 +345,7 @@ const runDiagnostics = (
nodes: {
node: ts.TaggedTemplateExpression | ts.StringLiteralLike;
schema: string | null;
tadaFragmentRefs?: readonly ts.Identifier[];
}[];
fragments: FragmentDefinitionNode[];
},
Expand All @@ -352,6 +354,7 @@ const runDiagnostics = (
): ts.Diagnostic[] => {
const filename = source.fileName;
const isCallExpression = info.config.templateIsCallExpression ?? true;
const typeChecker = info.languageService.getProgram()?.getTypeChecker();

const diagnostics = nodes
.map(originalNode => {
Expand Down Expand Up @@ -394,6 +397,20 @@ const runDiagnostics = (
(isExpression ? 2 : 0));
const endPosition = startingPosition + node.getText().length;
let docFragments = [...fragments];

if (originalNode.tadaFragmentRefs !== undefined) {
const fragmentNames = new Set<string>();
for (const identifier of originalNode.tadaFragmentRefs) {
const unrolled = unrollFragment(identifier, info, typeChecker);
unrolled.forEach((frag: FragmentDefinitionNode) =>
fragmentNames.add(frag.name.value)
);
}
docFragments = docFragments.filter(frag =>
fragmentNames.has(frag.name.value)
);
}

if (isCallExpression) {
try {
const documentFragments = parse(text, {
Expand Down
33 changes: 33 additions & 0 deletions test/e2e/fixture-project-tada/fixtures/missing-fragment-dep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { graphql } from './graphql';

const pokemonFragment = graphql(`
fragment PokemonBasicInfo on Pokemon {
id
name
}
`);

// This query correctly includes the fragment as a dep
const FirstQuery = graphql(
`
query FirstQuery {
pokemons(limit: 1) {
...PokemonBasicInfo
}
}
`,
[pokemonFragment]
);

// This query uses the fragment but DOES NOT include it as a dep
// It should show an error, but currently doesn't because the fragment
// was already added as a dep in FirstQuery above
const SecondQuery = graphql(`
query SecondQuery {
pokemons(limit: 2) {
...PokemonBasicInfo
}
}
`);

export { FirstQuery, SecondQuery };
71 changes: 71 additions & 0 deletions test/e2e/tada.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,3 +619,74 @@ List out all Pokémon, optionally in pages`
`);
}, 30000);
});

describe('Fragment dependencies - Issue #494', () => {
const projectPath = path.resolve(__dirname, 'fixture-project-tada');
const outfileMissingFragmentDep = path.join(
projectPath,
'missing-fragment-dep.ts'
);

let server: TSServer;
beforeAll(async () => {
server = new TSServer(projectPath, { debugLog: false });

server.sendCommand('open', {
file: outfileMissingFragmentDep,
fileContent: '// empty',
scriptKindName: 'TS',
} satisfies ts.server.protocol.OpenRequestArgs);

server.sendCommand('updateOpen', {
openFiles: [
{
file: outfileMissingFragmentDep,
fileContent: fs.readFileSync(
path.join(projectPath, 'fixtures/missing-fragment-dep.ts'),
'utf-8'
),
},
],
} satisfies ts.server.protocol.UpdateOpenRequestArgs);

server.sendCommand('saveto', {
file: outfileMissingFragmentDep,
tmpfile: outfileMissingFragmentDep,
} satisfies ts.server.protocol.SavetoRequestArgs);
});

afterAll(() => {
try {
fs.unlinkSync(outfileMissingFragmentDep);
} catch {}
});

it('warns about missing fragment dep even when fragment is used in another query in same file', async () => {
await server.waitForResponse(
e =>
e.type === 'event' &&
e.event === 'semanticDiag' &&
e.body?.file === outfileMissingFragmentDep
);

const res = server.responses.filter(
resp =>
resp.type === 'event' &&
resp.event === 'semanticDiag' &&
resp.body?.file === outfileMissingFragmentDep
);

// Should have a diagnostic about the unknown fragment in SecondQuery
expect(res.length).toBeGreaterThan(0);
expect(res[0].body.diagnostics.length).toBeGreaterThan(0);

const fragmentError = res[0].body.diagnostics.find((diag: any) =>
diag.text.includes('PokemonBasicInfo')
);

expect(fragmentError).toBeDefined();
expect(fragmentError.text).toBe('Unknown fragment "PokemonBasicInfo".');
expect(fragmentError.code).toBe(52001);
expect(fragmentError.category).toBe('error');
}, 30000);
});