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
15 changes: 12 additions & 3 deletions query-planner-js/src/buildQueryPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ import {
} from './QueryPlan';
import { getFieldDef, getResponseName } from './utilities/graphql';
import { MultiMap } from './utilities/MultiMap';
import { getFederationMetadataForType, getFederationMetadataForField } from './composedSchema';
import {
getFederationMetadataForType,
getFederationMetadataForField,
isValueTypeMetadata,
} from './composedSchema';
import { DebugLogger } from './utilities/debug';


Expand Down Expand Up @@ -1136,7 +1140,8 @@ export class QueryPlanningContext {
}

getBaseService(parentType: GraphQLObjectType): string | undefined {
return getFederationMetadataForType(parentType)?.graphName;
const type = getFederationMetadataForType(parentType);
return (type && !isValueTypeMetadata(type)) ? type.graphName : undefined;
}

getOwningService(
Expand Down Expand Up @@ -1170,7 +1175,11 @@ export class QueryPlanningContext {
});

for (const possibleType of this.getPossibleTypes(parentType)) {
const keys = getFederationMetadataForType(possibleType)?.keys?.get(serviceName);
const type = getFederationMetadataForType(possibleType);
const keys =
type && !isValueTypeMetadata(type)
? type.keys.get(serviceName)
: undefined;

if (!(keys && keys.length > 0)) continue;

Expand Down
14 changes: 12 additions & 2 deletions query-planner-js/src/composedSchema/buildComposedSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import { MultiMap } from '../utilities/MultiMap';
import {
FederationFieldMetadata,
FederationTypeMetadata,
FederationEntityTypeMetadata,
FieldSet,
GraphMap,
isValueTypeMetadata,
} from './metadata';

export function buildComposedSchema(document: DocumentNode): GraphQLSchema {
Expand Down Expand Up @@ -148,8 +150,11 @@ export function buildComposedSchema(document: DocumentNode): GraphQLSchema {
type.astNode,
);

// The assertion here guarantees the safety of the type cast below
// (typeMetadata as FederationEntityTypeMetadata). Adjustments to this assertion
// should account for this dependency.
assert(
!(typeMetadata.isValueType && typeDirectivesArgs.length >= 1),
!(isValueTypeMetadata(typeMetadata) && typeDirectivesArgs.length >= 1),
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved
`GraphQL type "${type.name}" cannot have a @${typeDirective.name} \
directive without an @${ownerDirective.name} directive`,
);
Expand All @@ -159,7 +164,12 @@ directive without an @${ownerDirective.name} directive`,

const keyFields = parseFieldSet(typeDirectiveArgs['key']);

typeMetadata.keys?.add(graphName, keyFields);
// We know we won't actually be looping here in the case of a value type
// based on the assertion above, but TS is not able to infer that.
(typeMetadata as FederationEntityTypeMetadata).keys.add(
graphName,
keyFields,
);
}

for (const fieldDef of Object.values(type.getFields())) {
Expand Down
23 changes: 19 additions & 4 deletions query-planner-js/src/composedSchema/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,25 @@ export type GraphMap = { [graphName: string]: Graph };
export interface FederationSchemaMetadata {
graphs: GraphMap;
}
export interface FederationTypeMetadata {
graphName?: GraphName;
keys?: MultiMap<GraphName, FieldSet>;
isValueType: boolean;

export type FederationTypeMetadata =
| FederationEntityTypeMetadata
| FederationValueTypeMetadata;

export interface FederationEntityTypeMetadata {
graphName: GraphName;
keys: MultiMap<GraphName, FieldSet>,
isValueType: false;
}

interface FederationValueTypeMetadata {
isValueType: true;
}

export function isValueTypeMetadata(
metadata: FederationTypeMetadata,
): metadata is FederationValueTypeMetadata {
return metadata.isValueType;
}

export interface FederationFieldMetadata {
Expand Down