From 22d5175455d8aab140a25e23d4d9c8dc10fadc50 Mon Sep 17 00:00:00 2001 From: Maarten Staa Date: Wed, 9 Sep 2020 14:41:53 +0200 Subject: [PATCH] Better support for abstract types (unions and interfaces). Represent the possible types and type names as unions in the Flow output, for more accurate typing. --- .../language/javascript/RelayFlowGenerator.js | 216 +++++++++++++----- .../RelayFlowGenerator-test.js.snap | 146 +++++------- 2 files changed, 211 insertions(+), 151 deletions(-) diff --git a/packages/relay-compiler/language/javascript/RelayFlowGenerator.js b/packages/relay-compiler/language/javascript/RelayFlowGenerator.js index 3c83199ac57dc..71eb5f24a07fd 100644 --- a/packages/relay-compiler/language/javascript/RelayFlowGenerator.js +++ b/packages/relay-compiler/language/javascript/RelayFlowGenerator.js @@ -53,7 +53,12 @@ import type { Selection as IRSelection, } from '../../core/IR'; import type {NodeVisitor} from '../../core/IRVisitor'; -import type {Schema, TypeID, EnumTypeID} from '../../core/Schema'; +import type { + Schema, + TypeID, + EnumTypeID, + UnionTypeID, +} from '../../core/Schema'; import type {RequiredDirectiveMetadata} from '../../transforms/RequiredFieldTransform'; import type {TypeGeneratorOptions} from '../RelayLanguagePluginInterface'; @@ -103,9 +108,16 @@ function makeProp( state: State, unmasked: boolean, concreteType?: string, + unionType?: UnionTypeID, ) { if (schemaName === '__typename' && concreteType) { value = t.stringLiteralTypeAnnotation(concreteType); + } else if (schemaName === '__typename' && unionType) { + value = t.unionTypeAnnotation( + schema + .getUnionTypes(unionType) + .map(type => t.stringLiteralTypeAnnotation(schema.getTypeString(type))), + ); } else if (nodeType) { value = transformScalarType( schema, @@ -113,6 +125,7 @@ function makeProp( state, selectionsToBabel( schema, + nodeType, [Array.from(nullthrows(nodeSelections).values())], state, unmasked, @@ -127,23 +140,40 @@ function makeProp( } const isTypenameSelection = selection => selection.schemaName === '__typename'; +const isFragmentSelection = (selection: Selection) => !!selection.ref; const hasTypenameSelection = selections => selections.some(isTypenameSelection); -const onlySelectsTypename = selections => selections.every(isTypenameSelection); +const onlySelectsFragments = (selections: Selection[]) => + selections.every(isFragmentSelection); function selectionsToBabel( schema: Schema, + nodeType: TypeID | null, selections: $ReadOnlyArray<$ReadOnlyArray>, state: State, unmasked: boolean, fragmentTypeName?: string, ) { const baseFields = new Map(); + const baseFragments = new Map(); const byConcreteType = {}; flattenArray(selections).forEach(selection => { - const {concreteType} = selection; + let {concreteType} = selection; + + // If the concrete type matches the node type, we can add this to the base fields + // and fragments instead. + if ( + nodeType && + concreteType && + schema.getTypeString(nodeType) === concreteType + ) { + concreteType = undefined; + } + if (concreteType) { byConcreteType[concreteType] = byConcreteType[concreteType] ?? []; byConcreteType[concreteType].push(selection); + } else if (selection.ref) { + baseFragments.set(selection.ref, selection); } else { const previousSel = baseFields.get(selection.key); @@ -154,21 +184,43 @@ function selectionsToBabel( } }); - const types = []; + // If there are any concrete types that only select fragments, move those + // fragments to the base fragments instead. + for (const concreteType in byConcreteType) { + const concreteTypeSelections = byConcreteType[concreteType]; + if (onlySelectsFragments(concreteTypeSelections)) { + concreteTypeSelections.forEach(selection => + baseFragments.set(selection.ref, selection), + ); - if ( + delete byConcreteType[concreteType]; + } + } + + const concreteTypes = []; + const typeFieldsPresentForUnion = Object.keys(byConcreteType).length > 0 && - onlySelectsTypename(Array.from(baseFields.values())) && (hasTypenameSelection(Array.from(baseFields.values())) || Object.keys(byConcreteType).every(type => hasTypenameSelection(byConcreteType[type]), - )) - ) { + )); + + if (typeFieldsPresentForUnion) { const typenameAliases = new Set(); for (const concreteType in byConcreteType) { - types.push( + const concreteTypeSelections = byConcreteType[concreteType]; + const concreteTypeSelectionsNames = concreteTypeSelections.map( + selection => selection.schemaName, + ); + + concreteTypes.push( groupRefs([ - ...Array.from(baseFields.values()), + // Deduplicate any fields also selected on the concrete type. + ...Array.from(baseFields.values()).filter( + selection => + isTypenameSelection(selection) && + !concreteTypeSelectionsNames.includes(selection.schemaName), + ), ...byConcreteType[concreteType], ]).map(selection => { if (selection.schemaName === '__typename') { @@ -178,24 +230,54 @@ function selectionsToBabel( }), ); } - // It might be some other type then the listed concrete types. Ideally, we - // would set the type to diff(string, set of listed concrete types), but - // this doesn't exist in Flow at the time. - types.push( - Array.from(typenameAliases).map(typenameAlias => { - const otherProp = readOnlyObjectTypeProperty( - typenameAlias, - t.stringLiteralTypeAnnotation('%other'), - ); - otherProp.leadingComments = lineComments( - "This will never be '%other', but we need some", - 'value in case none of the concrete values match.', - ); - return otherProp; - }), - ); - } else { - let selectionMap = selectionsToMap(Array.from(baseFields.values())); + + // It might be some other type then the listed concrete types. We try to + // figure out which types remain here. + let possibleTypesLeft: TypeID[] | null = null; + const innerType = + nodeType !== null ? schema.getNullableType(nodeType) : null; + if (innerType !== null && schema.isAbstractType(innerType)) { + const typesSeen = Object.keys(byConcreteType); + possibleTypesLeft = Array.from(schema.getPossibleTypes(innerType)).filter( + type => !typesSeen.includes(schema.getTypeString(type)), + ); + } + + // If we don't know which types are left we set the value to "%other", + // otherwise return a union of type names. + if (!possibleTypesLeft || possibleTypesLeft.length > 0) { + concreteTypes.push( + Array.from(typenameAliases).map(typenameAlias => { + const otherProp = readOnlyObjectTypeProperty( + typenameAlias, + possibleTypesLeft + ? t.unionTypeAnnotation( + possibleTypesLeft.map(type => + t.stringLiteralTypeAnnotation(schema.getTypeString(type)), + ), + ) + : t.stringLiteralTypeAnnotation('%other'), + ); + if (possibleTypesLeft) { + return otherProp; + } + + const otherPropWithComment = readOnlyObjectTypeProperty( + typenameAlias, + t.stringLiteralTypeAnnotation('%other'), + ); + otherPropWithComment.leadingComments = lineComments( + "This will never be '%other', but we need some", + 'value in case none of the concrete values match.', + ); + return otherPropWithComment; + }), + ); + } + } + + let selectionMap = selectionsToMap(Array.from(baseFields.values())); + if (!typeFieldsPresentForUnion) { for (const concreteType in byConcreteType) { selectionMap = mergeSelections( selectionMap, @@ -207,37 +289,56 @@ function selectionsToBabel( ), ); } - const selectionMapValues = groupRefs( - Array.from(selectionMap.values()), - ).map(sel => - isTypenameSelection(sel) && sel.concreteType - ? makeProp( - schema, - {...sel, conditional: false}, - state, - unmasked, - sel.concreteType, - ) - : makeProp(schema, sel, state, unmasked), - ); - types.push(selectionMapValues); } - return unionTypeAnnotation( - types.map(props => { - if (fragmentTypeName) { - props.push( - readOnlyObjectTypeProperty( - '$refType', - t.genericTypeAnnotation(t.identifier(fragmentTypeName)), - ), - ); - } - return unmasked - ? inexactObjectTypeAnnotation(props) - : exactObjectTypeAnnotation(props); - }), + const baseTypeProps = groupRefs( + [ + ...Array.from(baseFragments.values()), + ...Array.from(selectionMap.values()), + ].filter( + selection => + !typeFieldsPresentForUnion || !isTypenameSelection(selection), + ), + ).map(sel => + isTypenameSelection(sel) && + (sel.concreteType || + (nodeType && schema.isUnion(schema.getNullableType(nodeType)))) + ? makeProp( + schema, + {...sel, conditional: false}, + state, + unmasked, + sel.concreteType, + nodeType && schema.isUnion(schema.getNullableType(nodeType)) + ? schema.getNullableType(nodeType) + : undefined, + ) + : makeProp(schema, sel, state, unmasked), ); + + if (fragmentTypeName) { + baseTypeProps.push( + readOnlyObjectTypeProperty( + '$refType', + t.genericTypeAnnotation(t.identifier(fragmentTypeName)), + ), + ); + } + + const propsToObject = props => + unmasked + ? inexactObjectTypeAnnotation(props) + : exactObjectTypeAnnotation(props); + + const baseType = propsToObject(baseTypeProps); + if (concreteTypes.length === 0) { + return baseType; + } + + const unionType = unionTypeAnnotation(concreteTypes.map(propsToObject)); + return baseTypeProps.length > 0 + ? t.intersectionTypeAnnotation([unionType, baseType]) + : unionType; } function mergeSelection( @@ -346,6 +447,7 @@ function createVisitor( let responseTypeDefinition = selectionsToBabel( schema, + null, /* $FlowFixMe: selections have already been transformed */ (node.selections: $ReadOnlyArray<$ReadOnlyArray>), state, @@ -362,6 +464,7 @@ function createVisitor( `${node.name}Response`, selectionsToBabel( schema, + null, // $FlowFixMe[incompatible-cast] : selections have already been transformed (node.selections: $ReadOnlyArray<$ReadOnlyArray>), state, @@ -489,6 +592,7 @@ function createVisitor( const unmasked = node.metadata != null && node.metadata.mask === false; const baseType = selectionsToBabel( schema, + node.type, selections, state, unmasked, diff --git a/packages/relay-compiler/language/javascript/__tests__/__snapshots__/RelayFlowGenerator-test.js.snap b/packages/relay-compiler/language/javascript/__tests__/__snapshots__/RelayFlowGenerator-test.js.snap index bcf60588d52f0..2c9dfbb040a73 100644 --- a/packages/relay-compiler/language/javascript/__tests__/__snapshots__/RelayFlowGenerator-test.js.snap +++ b/packages/relay-compiler/language/javascript/__tests__/__snapshots__/RelayFlowGenerator-test.js.snap @@ -181,10 +181,6 @@ export type ConcreateTypes = {| |} | {| +__typename: "User", +name: ?string, - |} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other" |}), +$refType: ConcreateTypes$ref, |}; @@ -202,11 +198,6 @@ declare export opaque type PictureFragment$fragmentType: PictureFragment$ref; export type PictureFragment = {| +__typename: "Image", +$refType: PictureFragment$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: PictureFragment$ref, |}; export type PictureFragment$data = PictureFragment; export type PictureFragment$key = { @@ -237,11 +228,6 @@ declare export opaque type PageFragment$fragmentType: PageFragment$ref; export type PageFragment = {| +__typename: "Page", +$refType: PageFragment$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: PageFragment$ref, |}; export type PageFragment$data = PageFragment; export type PageFragment$key = { @@ -257,11 +243,6 @@ declare export opaque type UserFrag1$fragmentType: UserFrag1$ref; export type UserFrag1 = {| +__typename: "User", +$refType: UserFrag1$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: UserFrag1$ref, |}; export type UserFrag1$data = UserFrag1; export type UserFrag1$key = { @@ -277,11 +258,6 @@ declare export opaque type UserFrag2$fragmentType: UserFrag2$ref; export type UserFrag2 = {| +__typename: "User", +$refType: UserFrag2$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: UserFrag2$ref, |}; export type UserFrag2$data = UserFrag2; export type UserFrag2$key = { @@ -448,11 +424,6 @@ declare export opaque type SomeFragment$fragmentType: SomeFragment$ref; export type SomeFragment = {| +__typename: "User", +$refType: SomeFragment$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: SomeFragment$ref, |}; export type SomeFragment$data = SomeFragment; export type SomeFragment$key = { @@ -528,9 +499,7 @@ export type UnionTypeTestResponse = {| +__typename: "FakeNode", +id: string, |} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other" + +__typename: "NonNode" |}) |}; export type UnionTypeTest = {| @@ -2183,20 +2152,27 @@ export type RelayClientIDFieldQueryResponse = {| +__typename: string, +id: string, |}, - +node: ?{| - +__id: string, - +__typename: string, - +id: string, - +commentBody?: ?{| + +node: ?(({| + +__typename: "Comment", + +commentBody: ?(({| + +__typename: "PlainCommentBody", +__id: string, - +__typename: string, - +text?: ?{| + +text: ?{| +__id: string, +__typename: string, +text: ?string, |}, - |}, - |}, + |} | {| + +__typename: "MarkdownCommentBody" + |}) & {| + +__id: string + |}), + |} | {| + +__typename: "Feedback" | "Page" | "PhotoStory" | "Story" | "User" + |}) & {| + +__id: string, + +id: string, + |}), |}; export type RelayClientIDFieldQuery = {| variables: RelayClientIDFieldQueryVariables, @@ -2244,15 +2220,13 @@ fragment Foo on Node { import type { FragmentReference } from "relay-runtime"; declare export opaque type Foo$ref: FragmentReference; declare export opaque type Foo$fragmentType: Foo$ref; -export type Foo = ?({| +export type Foo = ?(({| +__typename: "User", +name: string, - +$refType: Foo$ref, |} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: Foo$ref, + +__typename: "Comment" | "Feedback" | "Page" | "PhotoStory" | "Story" +|}) & {| + +$refType: Foo$ref |}); export type Foo$data = Foo; export type Foo$key = { @@ -2777,10 +2751,6 @@ export type TypenameInsideWithOverlappingFields = {| +profile_picture: ?{| +uri: ?string |}, - |} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other" |}), +$refType: TypenameInsideWithOverlappingFields$ref, |}; @@ -2880,19 +2850,14 @@ fragment TypenameAliases on Actor { import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameInside$ref: FragmentReference; declare export opaque type TypenameInside$fragmentType: TypenameInside$ref; -export type TypenameInside = {| +export type TypenameInside = ({| +__typename: "User", +firstName: ?string, - +$refType: TypenameInside$ref, |} | {| +__typename: "Page", +username: ?string, - +$refType: TypenameInside$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: TypenameInside$ref, +|}) & {| + +$refType: TypenameInside$ref |}; export type TypenameInside$data = TypenameInside; export type TypenameInside$key = { @@ -2905,19 +2870,14 @@ export type TypenameInside$key = { import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameOutside$ref: FragmentReference; declare export opaque type TypenameOutside$fragmentType: TypenameOutside$ref; -export type TypenameOutside = {| +export type TypenameOutside = ({| +__typename: "User", +firstName: ?string, - +$refType: TypenameOutside$ref, |} | {| +__typename: "Page", +username: ?string, - +$refType: TypenameOutside$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +__typename: "%other", - +$refType: TypenameOutside$ref, +|}) & {| + +$refType: TypenameOutside$ref |}; export type TypenameOutside$data = TypenameOutside; export type TypenameOutside$key = { @@ -2930,15 +2890,21 @@ export type TypenameOutside$key = { import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameOutsideWithAbstractType$ref: FragmentReference; declare export opaque type TypenameOutsideWithAbstractType$fragmentType: TypenameOutsideWithAbstractType$ref; -export type TypenameOutsideWithAbstractType = {| - +__typename: string, +export type TypenameOutsideWithAbstractType = ({| + +__typename: "User", + +firstName: ?string, + +address: ?{| + +street: ?string, + +city: ?string, + |}, +|} | {| + +__typename: "Comment" | "Feedback" | "Page" | "PhotoStory" | "Story" +|}) & {| +username?: ?string, +address?: ?{| +city: ?string, +country: ?string, - +street?: ?string, |}, - +firstName?: ?string, +$refType: TypenameOutsideWithAbstractType$ref, |}; export type TypenameOutsideWithAbstractType$data = TypenameOutsideWithAbstractType; @@ -2953,8 +2919,8 @@ import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameWithoutSpreads$ref: FragmentReference; declare export opaque type TypenameWithoutSpreads$fragmentType: TypenameWithoutSpreads$ref; export type TypenameWithoutSpreads = {| - +firstName: ?string, +__typename: "User", + +firstName: ?string, +$refType: TypenameWithoutSpreads$ref, |}; export type TypenameWithoutSpreads$data = TypenameWithoutSpreads; @@ -2984,11 +2950,14 @@ export type TypenameWithoutSpreadsAbstractType$key = { import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameWithCommonSelections$ref: FragmentReference; declare export opaque type TypenameWithCommonSelections$fragmentType: TypenameWithCommonSelections$ref; -export type TypenameWithCommonSelections = {| - +__typename: string, +export type TypenameWithCommonSelections = ({| + +__typename: "User", + +firstName: ?string, +|} | {| + +__typename: "Page", + +username: ?string, +|}) & {| +name: ?string, - +firstName?: ?string, - +username?: ?string, +$refType: TypenameWithCommonSelections$ref, |}; export type TypenameWithCommonSelections$data = TypenameWithCommonSelections; @@ -3002,19 +2971,14 @@ export type TypenameWithCommonSelections$key = { import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameAlias$ref: FragmentReference; declare export opaque type TypenameAlias$fragmentType: TypenameAlias$ref; -export type TypenameAlias = {| +export type TypenameAlias = ({| +_typeAlias: "User", +firstName: ?string, - +$refType: TypenameAlias$ref, |} | {| +_typeAlias: "Page", +username: ?string, - +$refType: TypenameAlias$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +_typeAlias: "%other", - +$refType: TypenameAlias$ref, +|}) & {| + +$refType: TypenameAlias$ref |}; export type TypenameAlias$data = TypenameAlias; export type TypenameAlias$key = { @@ -3027,24 +2991,16 @@ export type TypenameAlias$key = { import type { FragmentReference } from "relay-runtime"; declare export opaque type TypenameAliases$ref: FragmentReference; declare export opaque type TypenameAliases$fragmentType: TypenameAliases$ref; -export type TypenameAliases = {| +export type TypenameAliases = ({| +_typeAlias1: "User", +_typeAlias2: "User", +firstName: ?string, - +$refType: TypenameAliases$ref, |} | {| +_typeAlias1: "Page", +_typeAlias2: "Page", +username: ?string, - +$refType: TypenameAliases$ref, -|} | {| - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +_typeAlias1: "%other", - // This will never be '%other', but we need some - // value in case none of the concrete values match. - +_typeAlias2: "%other", - +$refType: TypenameAliases$ref, +|}) & {| + +$refType: TypenameAliases$ref |}; export type TypenameAliases$data = TypenameAliases; export type TypenameAliases$key = {