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 #2394

I think @alloy PR #2293 could solve some of this issues

@jstejada any progress on that merge?