Skip to content

Commit

Permalink
Fix flow type generation for recursive input types
Browse files Browse the repository at this point in the history
Summary:
This is proposed as an alternative to D6770669

This is proposed as an alternative to #2142. Credit goes to benjaminogles for his original work in PR #2142 to fix this issue which ultimately led to this commit.

=== Motivation

Previously, if your GraphQL schema defined recursive input types, e.g.:

```
input InputA {
  field1: Int,
  fieldB: InputB,
}

input InputB {
  field1: String,
  fieldA: InputA,
}
```

The relay compiler would crash with a stack overflow error.

=== Solution

This was originally fixed in #2142 by determining recursive type definitions ahead of time and imposing a recursion limit on those fields.

This commit fixes the problem by generating separate flow types for input object types and referencing them recursively. This works because recursive types are allowed in flow.

The above example would produce the following flow type output:

```
export type InputA {
  field1: number,
  fieldB: InputBInputT,
}

export type InputB {
  field1: string,
  fieldA: InputAInputT,
}
```
Other types like `<>Variables` types that we generate also reference these new types.

== Caveats

- This is a bit wasteful because it will generate and export the same flow types in different generated flow definition files. I /think/ this isn't a major concern given that flow types don't affect runtime in any way. To prevent confusion about the duplicate types, we could consider not exporting them and just generating them as private types in the module.
- I /believe/ this shouldn't cause name conflicts given that these types are scoped by generated file.

Reviewed By: kassens

Differential Revision: D7022100

fbshipit-source-id: 10bfb80ca3e58ae8f98ed82ae6e87e7ba6a52b87
  • Loading branch information
Juan Tejada authored and facebook-github-bot committed Mar 5, 2018
1 parent c78e2b6 commit 6d49a52
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 18 deletions.
21 changes: 20 additions & 1 deletion packages/relay-compiler/core/RelayFlowGenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const PatchedBabelGenerator = require('./PatchedBabelGenerator');
const RelayMaskTransform = require('RelayMaskTransform');
const RelayRelayDirectiveTransform = require('RelayRelayDirectiveTransform');

const invariant = require('invariant');
const nullthrows = require('nullthrows');
const t = require('babel-types');

Expand All @@ -34,7 +35,7 @@ const {
transformScalarType,
transformInputType,
} = require('./RelayFlowTypeTransformers');
const {GraphQLNonNull} = require('graphql');
const {GraphQLInputObjectType, GraphQLNonNull} = require('graphql');
const {
FlattenTransform,
IRVisitor,
Expand All @@ -60,6 +61,9 @@ type Options = {|
export type State = {|
...Options,
+generatedFragments: Set<string>,
+generatedInputObjectTypes: {
[name: string]: GraphQLInputObjectType | 'pending',
},
+usedEnums: {[name: string]: GraphQLEnumType},
+usedFragments: Set<string>,
|};
Expand Down Expand Up @@ -230,6 +234,7 @@ function createVisitor(options: Options) {
enumsHasteModule: options.enumsHasteModule,
existingFragmentNames: options.existingFragmentNames,
generatedFragments: new Set(),
generatedInputObjectTypes: {},
inputFieldWhiteList: options.inputFieldWhiteList,
relayRuntimeModule: options.relayRuntimeModule,
usedEnums: {},
Expand All @@ -241,13 +246,15 @@ function createVisitor(options: Options) {
leave: {
Root(node) {
const inputVariablesType = generateInputVariablesType(node, state);
const inputObjectTypes = generateInputObjectTypes(state);
const responseType = exportType(
`${node.name}Response`,
selectionsToBabel(node.selections, state),
);
return t.program([
...getFragmentImports(state),
...getEnumDefinitions(state),
...inputObjectTypes,
inputVariablesType,
responseType,
]);
Expand Down Expand Up @@ -362,6 +369,18 @@ function flattenArray<T>(arrayOfArrays: Array<Array<T>>): Array<T> {
return result;
}

function generateInputObjectTypes(state: State) {
return Object.keys(state.generatedInputObjectTypes).map(typeIdentifier => {
const inputObjectType = state.generatedInputObjectTypes[typeIdentifier];
invariant(
typeof inputObjectType !== 'string',
'RelayCompilerFlowGenerator: Expected input object type to have been' +
' defined before calling `generateInputObjectTypes`',
);
return exportType(typeIdentifier, inputObjectType);
});
}

function generateInputVariablesType(node: Root, state: State) {
return exportType(
`${node.name}Variables`,
Expand Down
14 changes: 13 additions & 1 deletion packages/relay-compiler/core/RelayFlowTypeTransformers.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export type ScalarTypeMapping = {

import type {State} from './RelayFlowGenerator';

function getInputObjectTypeIdentifier(type: GraphQLInputObjectType): string {
return type.name;
}

function transformScalarType(
type: GraphQLType,
state: State,
Expand Down Expand Up @@ -108,6 +112,11 @@ function transformNonNullableInputType(type: GraphQLInputType, state: State) {
} else if (type instanceof GraphQLEnumType) {
return transformGraphQLEnumType(type, state);
} else if (type instanceof GraphQLInputObjectType) {
const typeIdentifier = getInputObjectTypeIdentifier(type);
if (state.generatedInputObjectTypes[typeIdentifier]) {
return t.identifier(typeIdentifier);
}
state.generatedInputObjectTypes[typeIdentifier] = 'pending';
const fields = type.getFields();
const props = Object.keys(fields)
.map(key => fields[key])
Expand All @@ -122,7 +131,10 @@ function transformNonNullableInputType(type: GraphQLInputType, state: State) {
}
return property;
});
return t.objectTypeAnnotation(props);
state.generatedInputObjectTypes[typeIdentifier] = t.objectTypeAnnotation(
props,
);
return t.identifier(typeIdentifier);
} else {
throw new Error(`Could not convert from GraphQL type ${type.toString()}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,19 @@ mutation CommentCreateMutation(
}
~~~~~~~~~~ OUTPUT ~~~~~~~~~~
export type CommentCreateInput = {
clientMutationId?: ?string,
feedbackId?: ?string,
feedback?: ?CommentfeedbackFeedback,
};
export type CommentfeedbackFeedback = {
comment?: ?FeedbackcommentComment,
};
export type FeedbackcommentComment = {
feedback?: ?CommentfeedbackFeedback,
};
export type CommentCreateMutationVariables = {|
input: {
clientMutationId?: ?string,
feedbackId?: ?string,
},
input: CommentCreateInput,
first?: ?number,
orderBy?: ?$ReadOnlyArray<string>,
|};
Expand Down Expand Up @@ -329,11 +337,12 @@ mutation InputHasArray($input: UpdateAllSeenStateInput) {
}
~~~~~~~~~~ OUTPUT ~~~~~~~~~~
export type UpdateAllSeenStateInput = {
clientMutationId?: ?string,
storyIds?: ?$ReadOnlyArray<?string>,
};
export type InputHasArrayVariables = {|
input?: ?{
clientMutationId?: ?string,
storyIds?: ?$ReadOnlyArray<?string>,
},
input?: ?UpdateAllSeenStateInput,
|};
export type InputHasArrayResponse = {|
+viewerNotificationsUpdateAllSeenState: ?{|
Expand Down Expand Up @@ -423,11 +432,19 @@ export type ExampleFragment = {|
+$refType: ExampleFragment$ref,
|};
export type CommentCreateInput = {
clientMutationId?: ?string,
feedbackId?: ?string,
feedback?: ?CommentfeedbackFeedback,
};
export type CommentfeedbackFeedback = {
comment?: ?FeedbackcommentComment,
};
export type FeedbackcommentComment = {
feedback?: ?CommentfeedbackFeedback,
};
export type TestMutationVariables = {|
input: {
clientMutationId?: ?string,
feedbackId?: ?string,
},
input: CommentCreateInput,
|};
export type TestMutationResponse = {|
+commentCreate: ?{|
Expand All @@ -437,11 +454,12 @@ export type TestMutationResponse = {|
|},
|};
export type FeedbackLikeInput = {
clientMutationId?: ?string,
feedbackId?: ?string,
};
export type TestSubscriptionVariables = {|
input?: ?{
clientMutationId?: ?string,
feedbackId?: ?string,
},
input?: ?FeedbackLikeInput,
|};
export type TestSubscriptionResponse = {|
+feedbackLikeSubscribe: ?{|
Expand Down
9 changes: 9 additions & 0 deletions packages/relay-test-utils/testschema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ input ApplicationRequestDeleteAllInput {
input CommentCreateInput {
clientMutationId: String
feedbackId: ID
feedback: CommentfeedbackFeedback
}

input CommentfeedbackFeedback {
comment: FeedbackcommentComment
}

input FeedbackcommentComment {
feedback: CommentfeedbackFeedback
}

input CommentCreateSubscriptionInput {
Expand Down

0 comments on commit 6d49a52

Please sign in to comment.