Relay: Nested fragments cause flow errors with relay modern

[cdriscol]

I'm currently experiencing an issue with flow generated prop types where I'm trying to use flow to set my prop types but I keep getting missing field errors for components that define fragments. It looks like relay-compiler (as of version 1.5+) is generating the correct types with fragmentRef's, but they do not appear to be working as I would expect.

I used the todo-mvc app to show my problem: https://github.com/cdriscol/relay-todomvc/tree/found-modern-flow

I essentially have the following changes:

// @flow
// TodoApp.js
// ...
import type { TodoApp_viewer } from './__generated__/TodoApp_viewer.graphql';
type Props = {
    viewer: TodoApp_viewer,
    children: any,
    relay: any
};

class TodoApp extends React.Component<Props> {
// ...
<TodoListFooter viewer={viewer} />
// ...
}

// @flow
// TodoListFooter.js
//....
import type { TodoListFooter_viewer } from './__generated__/TodoListFooter_viewer.graphql';

type Props = {
  viewer: TodoListFooter_viewer,
  relay: any,
};

class TodoListFooter extends React.Component<Props> {
//...
}

What I'm seeing is 5 errors on the TodoApp.js, one of which is the following..

Error: src/components/TodoApp.js:42
 42:           <TodoListFooter viewer={viewer} />
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ props. This type is incompatible with
 14: class TodoListFooter extends React.Component<Props> {
                                                  ^^^^^ Props. See: src/components/TodoListFooter.js:14
  Property `viewer` is incompatible:
     42:           <TodoListFooter viewer={viewer} />
                                           ^^^^^^ TodoApp_viewer. This type is incompatible with
     10:   viewer: TodoListFooter_viewer,
                   ^^^^^^^^^^^^^^^^^^^^^ TodoListFooter_viewer. See: src/components/TodoListFooter.js:10
      Property `__fragments` is incompatible:
         42:           <TodoListFooter viewer={viewer} />
                                               ^^^^^^ property `__fragments`. Property not found in
         10:   viewer: TodoListFooter_viewer,
                       ^^^^^^^^^^^^^^^^^^^^^ TodoListFooter_viewer. See: src/components/TodoListFooter.js:10

I'm wondering why flow cannot handle this case, as the generated flow types do seem to look correct and reference the correct fragment.

// TodoApp_viewer.graphql.js
/*::
import type { ConcreteFragment } from 'relay-runtime';
type TodoListFooter_viewer$ref = any;
import type { FragmentReference } from 'relay-runtime';
export opaque type TodoApp_viewer$ref: FragmentReference = FragmentReference;
export type TodoApp_viewer = {|
  +id: string,
  +__fragments: TodoListFooter_viewer$ref,
  +$refType: TodoApp_viewer$ref,
|};
*/

// TodoListFooter_viewer.graphql.js
/*::
import type { ConcreteFragment } from 'relay-runtime';
import type { FragmentReference } from 'relay-runtime';
export opaque type TodoListFooter_viewer$ref: FragmentReference = FragmentReference;
export type TodoListFooter_viewer = {|
  +todos: ?{|
    +edges: ?$ReadOnlyArray<?{|
      +node: ?{|
        +id: string,
        +complete: ?boolean,
      |},
    |}>,
  |},
  +id: string,
  +numTodos: ?number,
  +numCompletedTodos: ?number,
  +$refType: TodoListFooter_viewer$ref,
|};
*/

Sorry for the long explanation and lots of code pasting, I just can't seem to figure out why flow is complaining about the generated types.. maybe I'm missing something here?

[alloy]

This is because properly typing fragment references currently only happens when you use the Haste module system. If you’re not at FB, then you’re probably not using that. The issue is that without the Haste module system, it’s hard to import fragment types of your child fragments using the current scheme of colocating artifacts next to the source files (paths are not stable, you may move files around). In short, this is why your fragment ref is typed as any, rather than being imported from the child fragment artifact.

I don’t use Flow and would have expected the any type to make it at least not complain, but I guess not.

I do use TypeScript and some of us have been working on making Relay-compiler work with that. As part of that effort we also decided to add the option to store all artifacts in a single directory, which enables behavior similarly to using the Haste module system. The PR for it is here and you can read more about the artifactDirectory option in this repo https://github.com/relay-tools/relay-compiler-language-typescript

Perhaps you can give that a spin and see if it indeed makes the lives of Flow users easier as well (which we expect, but haven’t yet verified).

[kikoseijo]

For me "any" its like a wildcard, you don't benefit of the autocompletion check if you use any, thats the purpose for it.

What I ended up doing its replicate the models as types and just import myself instead of going into the generated folder.

Been trying to figure out how to take the advantage of the GraphQL schema, as them type definitions could be handy on flow, but could not find any info and I ended up writing the types for my models.

[Nodonisko]

This is little bit pain, when you want 100% typesafe app. Any ideas?

[kikoseijo]

Dont use the __generated__ type definitions.
Use your own types, which are basically a copy of the schema types.

Works great fo me!

[alloy]

@cdriscol Were you able to pin point the issue since then?

[steida]

The best workaround as far I know.

[irrigator]

I'm pretty sure version 1.4 does not have this issue.

[sibelius]

we should learn and document how to use these 2 helpers

$FragmentRef: https://github.com/facebook/relay/blob/master/packages/react-relay/modern/ReactRelayTypes.js#L92

and

$RelayProps: https://github.com/facebook/relay/blob/master/packages/react-relay/modern/ReactRelayTypes.js#L101

[steida]

@sibelius Can you send an example to Este please? Thank you.

[istarkov]

@sibelius the main problem that it seems like opaque types defined in one file like

declare export opaque type Blabla_name$ref: FragmentReference;

is not somehow associated with same reference in other file

type Blabla_name$ref = any; <-- this Blabla_name$ref is not the same as opaque Blabla_name$ref
...
+$fragmentRefs: Blabla_name$ref

so in this case flow read it as +$fragmentRefs: any and accept anything

Does anybody knew how to link that refs together?

[istarkov]

And the main stopper is here https://github.com/facebook/relay/blob/master/packages/relay-compiler/core/RelayFlowGenerator.js#L442
see todo, will try to fix via flowconfig

[istarkov]

Can be used now, if change this https://github.com/facebook/relay/blob/master/packages/relay-compiler/bin/RelayCompilerBin.js#L220 useHaste: true
Then add all your relay generated folders into

# .flowconfig
[options]
module.system.node.resolve_dirname=./blbal/__generated__
module.system.node.resolve_dirname=./blbalbla/__generated__
...

We can use FragmentRef as expected, so there are 2 way to achieve result.
1) Fix todo in link above
2) Allow pass useHaste true, and add locally __generated__ dirs to .flowconfig

[sibelius]

related to https://github.com/facebook/relay/issues/2394

I think @alloy PR https://github.com/facebook/relay/pull/2293 could solve some of this issues

@jstejada any progress on that merge?

[alloy]

Yup, #2293 will allow fragment reference type checking without using haste 👍

[bastoche]

There is actually a workaround described in the relay doc:

Applied to a fragment definition, @relay(mask: false) changes the generated Flow types to be better usable when the fragment is included with the same directive. The Flow types will no longer be exact objects and no longer contain internal marker fields.

To be clear, you need to add @relay(mask: false) both when defining the fragment:

fragment Todo_todo on Todo @relay(mask: false)

and when using it:

...Todo_todo @relay(mask: false)

You will find a working example in this repository.

[steida]

@bastoche That's very suboptimal.

@alloy What I can do to help?

[alloy]

@steida Seeing as my PR that has the single artefact dir feature is now merged and available in the 1.7.0 RC1, it would be great if you could give that version a try and let me know if it works well for you! (See the TS plugin for a configuration example https://github.com/relay-tools/relay-compiler-language-typescript#configuration, which I now realise wasn’t added to the Relay docs, so if everything works out we should document that here.)

[steida]

@alloy So I tested it, and it seems nothing has changed. I am not using typescript but Flow. Should I do the same for Flow?

Maybe related to https://github.com/facebook/relay/issues/2516

@kassens Should it work OOTB or some another action is required? Thank you.

[alloy]

@steida Yes, you should follow the same configuration steps, to get a single artefact directory, for Flow to get fragment references to work. You can find some more explanation here and https://github.com/facebook/relay/issues/2509.

[steida]

@alloy OK, I updated the code. It seems it does what it should, but also it seems we need some magic with $FragmentRef.

[steida]

There is some Relay black magic I can't figure out how it is supposed to work. Basically, createFragmentContainer lies about what it needs. It needs +$fragmentRefs but it requires wrapped component props.

[steida]

@kassens Is flow-typed libdefs wrong? Or should we use any recasting? Or something else? It must be one liner, please share it with us. Thank you.

[richardgirges]

I upgraded to Flow 0.80.0 and this issue disappeared off the face of the planet. Seems too good to be true. I was originally going with @steida's suggestion, using $FlowFixMe comments everywhere, now they're all yielding "unused suppression" warnings.

Can anyone else confirm that nested fragment defs are working in Flow 0.80.0?

[steida]

Probably some error at your side. It still does not work.

[richardgirges]

Apologies, I spoke too soon.

The majority of the nested fragment references actually were fixed on my side when I upgraded to flow 0.80.0.

But there are still a few straggling areas where I’m getting the same flow errors and the suppression is still in effect.

Haven’t bothered to understand why some references are working and others aren’t.

[willisplummer]

@steida are you still experiencing this problem? i wasn't able to get it to work even using artifacts directory

[sibelius]

Try with typescript https://medium.com/@sibelius/relay-modern-migration-to-typescript-c26ab0ee749c

[willisplummer]

@sibelius 🤔 unlikely that we'll switch type systems at work and i assume that there is a way to get this working with flow given that facebook maintains and uses both tools in conjunction.

Edit: I was able to resolve my issues by updating the type definitions from flow-typed based on @mrtnzlml's comment here: https://github.com/facebook/relay/issues/2516#issuecomment-434984019

[RichardLindhout]

I'm still having this problem when using fragments and I want to generate some tree data in the upper level of all components with the data from sub fragments

I would expect the type to be:
readonly flowBlocks: ReadonlyArray<FlowBlock_block>;

but it is

readonly flowBlocks: ReadonlyArray<{
        readonly " $fragmentRefs": FlowBlock_flowBlock$ref;
    }>;

[RichardLindhout]

Ah, probably need the ReadonlyArray as a type in my code instead of ReadonlyArray 👍
thanks @sibelius

[jnak]

The issue is still happening even though the documentation says it works. It's most likely because the flow-typed definitions for createFragmentContainer are incorrect. Since it's working correctly at Facebook, it would be much appreciated if someone working at Facebook could share the flow definitions Facebook uses. Thanks.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[simkessy]

Im seeing this issue on latest relay modern using hooks, is there a recommended solution?

[jstejada]

@simkessy could you share exactly what Flow error are you seeing with hooks and an example of your code that produces the error?