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

Improves error messages when an empty key is generated #30

Merged
merged 2 commits into from
Aug 20, 2024
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to
[Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [v3.2.2] - 2024-08-20

### Fixed

- Improves the error message surfaced in cases where FROID generates an empty
key.

## [v3.2.1] - 2024-04-29

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@wayfair/node-froid",
"version": "3.2.1",
"version": "3.2.2",
"description": "Federated GQL Relay Object Identification implementation",
"main": "dist/index.js",
"types": "dist/index.d.ts",
Expand Down
31 changes: 25 additions & 6 deletions src/schema/Key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
parseableField = Key.getKeyDirectiveFields(parseableField);
}
(
Key.parseKeyFields(parseableField)
Key.parseKeyFields(this.typename, parseableField)
.definitions[0] as OperationDefinitionNode
)?.selectionSet?.selections?.forEach((selection) =>
this.addSelection(selection)
Expand Down Expand Up @@ -174,6 +174,7 @@
*/
public toString(): string {
return Key.getSortedSelectionSetFields(
this.typename,
this._fields.map((field) => field.toString()).join(' ')
);
}
Expand Down Expand Up @@ -209,11 +210,23 @@
/**
* Parses a key fields string into AST.
*
* @param {string} keyFields - The key fields string

Check warning on line 213 in src/schema/Key.ts

View workflow job for this annotation

GitHub Actions / lint

Expected @param names to be "typename, keyFields". Got "keyFields, typename"
* @param {string} typename - The typename of the node the directive belongs to
* @returns {DocumentNode} The key fields represented in AST
*/
private static parseKeyFields(keyFields: string): DocumentNode {
return parse(`{${keyFields}}`, {noLocation: true});
private static parseKeyFields(
typename: string,
keyFields: string
): DocumentNode {
try {
return parse(`{${keyFields}}`, {noLocation: true});
} catch (error) {
throw new Error(
`Failed to parse key fields "${keyFields}" for type "${typename}" due to error: ${
(error as Error).message
}`
);
}
}

/**
Expand All @@ -233,13 +246,19 @@
/**
* Sorts the selection set fields.
*
* @param {string} fields - The selection set fields.

Check warning on line 249 in src/schema/Key.ts

View workflow job for this annotation

GitHub Actions / lint

Expected @param names to be "typename, fields". Got "fields, typename"
* @param {string} typename - The typename of the node the directive belongs to
* @returns {string} The sorted selection set fields.
*/
public static getSortedSelectionSetFields(fields: string): string {
public static getSortedSelectionSetFields(
typename: string,
fields: string
): string {
const selections = Key.sortSelectionSetByNameAscending(
(Key.parseKeyFields(fields).definitions[0] as OperationDefinitionNode)
.selectionSet
(
Key.parseKeyFields(typename, fields)
.definitions[0] as OperationDefinitionNode
).selectionSet
);
return Key.formatSelectionSetFields(print(selections));
}
Expand Down
25 changes: 25 additions & 0 deletions src/schema/__tests__/FroidSchema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3009,6 +3009,31 @@ describe('FroidSchema class', () => {
);
});

it('provides a helpful message when failing to stringify the FROID schema', () => {
const productSchema = gql`
type Product @key(fields: "__typename") {
name: String
}
`;
const subgraphs = new Map();
subgraphs.set('product-subgraph', productSchema);

let errorMessage;
try {
generateSchema({
subgraphs,
froidSubgraphName: 'relay-subgraph',
federationVersion: FED2_DEFAULT_VERSION,
});
} catch (error) {
errorMessage = error.message;
}

expect(errorMessage).toMatch(
'Failed to parse key fields "" for type "Product"'
);
});

it('generates schema document AST', () => {
const productSchema = gql`
type Product @key(fields: "upc") {
Expand Down
10 changes: 8 additions & 2 deletions src/schema/sortDocumentAst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ function sortChildren(node: NamedNode): NamedNode {
}

const directives = node?.directives
? {directives: sortDirectives(sortKeyDirectiveFields(node.directives))}
? {
directives: sortDirectives(
sortKeyDirectiveFields(node.name.value, node.directives)
),
}
: {};

if (
Expand Down Expand Up @@ -254,10 +258,12 @@ function sortChildren(node: NamedNode): NamedNode {
/**
* Sorts the `fields` argument of any @key directives in a list of directives.
*
* @param {string} typename - The typename of the node the directive belongs to
* @param {ConstDirectiveNode[]} directives - A list of directives that may include the @key directive
* @returns {ConstDirectiveNode[]} The list of directives after any key fields have been sorted
*/
function sortKeyDirectiveFields(
typename: string,
directives: readonly ConstDirectiveNode[]
): readonly ConstDirectiveNode[] {
return directives.map((directive) => {
Expand Down Expand Up @@ -285,7 +291,7 @@ function sortKeyDirectiveFields(
...fieldsArgument,
value: {
...fieldsArgument.value,
value: Key.getSortedSelectionSetFields(fields),
value: Key.getSortedSelectionSetFields(typename, fields),
},
},
],
Expand Down
Loading