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

chore: typescript-redux followups #656

Merged
merged 21 commits into from
Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e715ab6
Refine / split metadata types (instead of ? -> !)
trevor-scheer Apr 8, 2021
bdad209
getJoins -> getJoinDefinitions
trevor-scheer Apr 8, 2021
b3bb32e
sanitizedServiceNames -> graphNameToEnumValueName
trevor-scheer Apr 8, 2021
afd924c
Address enum sanitization/uniquification comments
trevor-scheer Apr 12, 2021
5bd3f10
Use actual map for GraphMap instead to account for undefined-ness
trevor-scheer Apr 13, 2021
4378987
Clean up usages of printWithReducedWhitespace in favor of stripIgnore…
trevor-scheer Apr 13, 2021
5e40df8
Confirm parsed FieldSets do not have an injected operation
trevor-scheer Apr 15, 2021
e37e2a6
Ensure no FragmentSpreads nested in a FieldSet
trevor-scheer Apr 15, 2021
2aa3cf5
Capture caveats in comments from commit messages
trevor-scheer Apr 15, 2021
fb0f6e9
Remove incorrect nullish coalesce to ownerService
trevor-scheer Apr 15, 2021
5ac14f2
Update ordering of join__Graph enum in test mocks
trevor-scheer Apr 16, 2021
7784276
Invert metadata predicate which was always negated to its opposite
trevor-scheer Apr 21, 2021
4b7d50f
Update expectations comment
trevor-scheer Apr 21, 2021
6c4261b
Create nice helper for working with Maps (mapGetOrSet)
trevor-scheer Apr 21, 2021
524809d
Fix usage of mapGetOrSet
trevor-scheer Apr 21, 2021
208a041
Add clarity to names
trevor-scheer Apr 21, 2021
f85a597
Correct error message
trevor-scheer Apr 21, 2021
2a2c429
Simplify extra } error message
trevor-scheer Apr 21, 2021
a1156a8
Fix remaining accesses to context.graphNameToEnumValueName
trevor-scheer Apr 21, 2021
53e0ad5
Update changelogs
trevor-scheer Apr 22, 2021
fe2c2aa
Merge branch 'main' into trevor/redux-followups
trevor-scheer Apr 22, 2021
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
34 changes: 31 additions & 3 deletions federation-js/src/composition/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,37 @@ function removeExternalFieldsFromExtensionVisitor<
};
}

export function parseSelections(source: string) {
return (parse(`query { ${source} }`)
.definitions[0] as OperationDefinitionNode).selectionSet.selections;
/**
* For lack of a "home of federation utilities", this function is copy/pasted
* verbatim across the federation, gateway, and query-planner packages. Any changes
* made here should be reflected in the other two locations as well.
*
* @param condition
* @param message
* @throws
*/
export function assert(condition: any, message: string): asserts condition {
if (!condition) {
throw new Error(message);
}
}

/**
* For lack of a "home of federation utilities", this function is copy/pasted
* verbatim across the federation, gateway, and query-planner packages. Any changes
* made here should be reflected in the other two locations as well.
*
* @param source A string representing a FieldSet
* @returns A parsed FieldSet
*/
export function parseSelections(source: string): ReadonlyArray<SelectionNode> {
const parsed = parse(`{${source}}`);
assert(
parsed.definitions.length === 1,
`Invalid FieldSet provided: '${source}'. FieldSets may not contain operations within them.`,
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved
);
return (parsed.definitions[0] as OperationDefinitionNode).selectionSet
.selections;
}

export function hasMatchingFieldInDirectives({
Expand Down
14 changes: 14 additions & 0 deletions gateway-js/src/utilities/assert.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* For lack of a "home of federation utilities", this function is copy/pasted
* verbatim across the federation, gateway, and query-planner packages. Any changes
* made here should be reflected in the other two locations as well.
*
* @param condition
* @param message
* @throws {@link Error}
*/
export function assert(condition: any, message: string): asserts condition {
if (!condition) {
throw new Error(message);
}
}
18 changes: 16 additions & 2 deletions gateway-js/src/utilities/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
SelectionNode,
TypeNode,
} from 'graphql';
import { assert } from './assert';

export function getResponseName(node: FieldNode): string {
return node.alias ? node.alias.value : node.name.value;
Expand Down Expand Up @@ -41,7 +42,20 @@ export function astFromType(type: GraphQLType): TypeNode {
}
}

/**
* For lack of a "home of federation utilities", this function is copy/pasted
* verbatim across the federation, gateway, and query-planner packages. Any changes
* made here should be reflected in the other two locations as well.
*
* @param source A string representing a FieldSet
* @returns A parsed FieldSet
*/
export function parseSelections(source: string): ReadonlyArray<SelectionNode> {
return (parse(`query { ${source} }`)
.definitions[0] as OperationDefinitionNode).selectionSet.selections;
const parsed = parse(`{${source}}`);
assert(
parsed.definitions.length === 1,
`Invalid FieldSet provided: '${source}'. FieldSets may not contain operations within them.`,
);
return (parsed.definitions[0] as OperationDefinitionNode).selectionSet
.selections;
}
28 changes: 28 additions & 0 deletions query-planner-js/src/utilities/__tests__/graphql.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { FieldNode } from 'graphql';
import { parseSelections } from '../graphql';

describe('graphql utility functions', () => {
describe('parseSelections', () => {
it('parses a valid FieldSet', () => {
const fieldSet = 'foo bar';
const parsed = parseSelections(fieldSet);
expect(parsed).toHaveLength(2);
});

it('parses a nested FieldSet', () => {
const fieldSet = 'foo { bar }';
const parsed = parseSelections(fieldSet);
expect(parsed).toHaveLength(1);
expect((parsed[0] as FieldNode).selectionSet?.selections).toHaveLength(1);
});

it('throws when injecting an extra operation', () => {
const invalidFieldSet = 'foo } query X { bar';
expect(() =>
parseSelections(invalidFieldSet),
).toThrowErrorMatchingInlineSnapshot(
`"Invalid FieldSet provided: 'foo } query X { bar'. FieldSets may not contain operations within them."`,
);
});
});
});
9 changes: 9 additions & 0 deletions query-planner-js/src/utilities/assert.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
/**
* For lack of a "home of federation utilities", this function is copy/pasted
* verbatim across the federation, gateway, and query-planner packages. Any changes
* made here should be reflected in the other two locations as well.
*
* @param condition
* @param message
* @throws
*/
export function assert(condition: any, message: string): asserts condition {
if (!condition) {
throw new Error(message);
Expand Down
23 changes: 15 additions & 8 deletions query-planner-js/src/utilities/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
parse,
SchemaMetaFieldDef,
SelectionNode,
SelectionSetNode,
TypeMetaFieldDef,
TypeNameMetaFieldDef,
TypeNode,
Expand Down Expand Up @@ -96,14 +95,22 @@ export function astFromType(type: GraphQLType): TypeNode {
}
}

export function parseSelectionSet(source: string): SelectionSetNode {
return (parse(`query ${source}`)
.definitions[0] as OperationDefinitionNode).selectionSet;
}

/**
* For lack of a "home of federation utilities", this function is copy/pasted
* verbatim across the federation, gateway, and query-planner packages. Any changes
* made here should be reflected in the other two locations as well.
*
* @param source A string representing a FieldSet
* @returns A parsed FieldSet
*/
export function parseSelections(source: string): ReadonlyArray<SelectionNode> {
return (parse(`query { ${source} }`)
.definitions[0] as OperationDefinitionNode).selectionSet.selections;
const parsed = parse(`{${source}}`);
Copy link
Member

Choose a reason for hiding this comment

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

Could be an opportunity to provide a more useful error message in the case of errors vai this parseSelections function, or possibly some specialized utility functions that produce valuable error messages in the event of parse errors; I'm just going to put this on the same radar by linking to this:

#679

e.g., perhaps a parseKeys, parseRequires, parseProvides might wrap this, or perhaps this can take a param.

Just a suggestion, not asking for it in this PR.

assert(
parsed.definitions.length === 1,
`Invalid FieldSet provided: '${source}'. FieldSets may not contain operations within them.`,
);
return (parsed.definitions[0] as OperationDefinitionNode).selectionSet
.selections;
}

// Using `getArgumentValues` from `graphql-js` ensures that arguments are of the right type,
Expand Down