Skip to content

$RelayProps flow type is incorrect #2443

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

Closed
roballsopp opened this issue May 8, 2018 · 5 comments
Closed

$RelayProps flow type is incorrect #2443

roballsopp opened this issue May 8, 2018 · 5 comments

Comments

@roballsopp
Copy link

The flow type defined for $RelayProps is:

export type $RelayProps<Props, RelayPropT> = $ObjMap<
  $Diff<Props, {relay: RelayPropT | void}>,
  & (<T: {+$refType: empty}>( T) =>  T)
  & (<T: {+$refType: empty}>(?T) => ?T)
  & (<TRef: FragmentReference, T: {+$refType: TRef}>(                 T ) =>                  $FragmentRef<T> )
  & (<TRef: FragmentReference, T: {+$refType: TRef}>(?                T ) => ?                $FragmentRef<T> )
  & (<TRef: FragmentReference, T: {+$refType: TRef}>( $ReadOnlyArray< T>) =>  $ReadOnlyArray< $FragmentRef<T>>)
  & (<TRef: FragmentReference, T: {+$refType: TRef}>(?$ReadOnlyArray< T>) => ?$ReadOnlyArray< $FragmentRef<T>>)
  & (<TRef: FragmentReference, T: {+$refType: TRef}>( $ReadOnlyArray<?T>) =>  $ReadOnlyArray<?$FragmentRef<T>>)
  & (<TRef: FragmentReference, T: {+$refType: TRef}>(?$ReadOnlyArray<?T>) => ?$ReadOnlyArray<?$FragmentRef<T>>)
  & (<T>(T) => T),
>;

A contrived usage might be like:

import { RelayProp } from 'react-relay';
import type { someFragment } from './__generated__/someFragment.graphql';

type Props = {
  someProp: someFragment,
  someOtherProp: { foo: string },
  relay: RelayProp
};

type ModifiedProps = $RelayProps<Props, RelayProp>

I think the intent is that ModifiedProps will come out like:

type ModifiedProps = {
  someProp: FragmentReference,
  someOtherProp: { foo: string },
}

That is, anything with a $refType property (anything thats a graphql fragment) should be converted from its full type given by the generated file into a $FragmentRef<T>. However, it looks like the two {+$refType: empty} lines in the $RelayProps type might be defeating this, and simply returning the original type of each fragment prop. This is causing numerous missing property errors when using createFragmentContainer (#2394, #2316).

Is there a reason for the {+$refType: empty} lines? Is the fix simply to remove them? Removing them in my own code seems work perfectly, and my createFragmentContainer components are now typed correctly.

@kassens
Copy link
Member

kassens commented May 8, 2018

If the component prop type is any or Function, we don't want to match the $FragmentRef<T> cases.

I think the problem is that we're not generating the correct flow types in non-haste environments 👎

This is the problem: https://github.com/facebook/relay/blob/master/packages/relay-compiler/core/RelayFlowGenerator.js#L443
Essentially currently generating {+$refType: any}> which is the one thing matching {+$refType: empty}> (nothing matches empty except for any which matches absolutely anything).

The difficulty is not knowing where to import the fragment from.

@sibelius
Copy link
Contributor

sibelius commented May 8, 2018

@kassens non-haste environments is fixed by @alloy here #2293

@sibelius
Copy link
Contributor

sibelius commented Jun 4, 2018

for now you can enable haste environment on .flowconfig and this will work out fine

https://flow.org/en/docs/config/options/#toc-module-system-node-haste

@leethree
Copy link

what is the correct way to use $RelayProps? is there a complete example?

@kassens
Copy link
Member

kassens commented Aug 16, 2018

$RelayProps is used internally for createFragmentContainer, the empty case is a special case to make sure Function and Object are not treated as having a $ref property (Flow treats those unsafe types as having any possible fields).

I opened #2516 to document the flow usage better, but pretty sure there are no fundamental issues with the types as we use them everywhere internally at Facebook.

@kassens kassens closed this as completed Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants