DefinitelyTyped Relay typings with fragment reference support

[alloy]

Filing a ticket here to keep track of this external work https://github.com/alloy/DefinitelyTyped/pull/1

[kastermester]

This can also be closed I think?

[alloy]

This is the work that still needs to be done once TS 2.8 is released.

[kastermester]

Ah ok.

Just for future reference (and potentially others reading)

We want to transform properties that originate from fragment specifications, such the following works:

import { MyParentContainer_data } from '__generated__/MyParentContainer_data.graphql';
const MyParentComponent = (props: Props) => <MyChildContainer data={props.data} />;
const MyParentContainer = createFragmentContainer(MyParentComponent, graphql`fragment MyParentContainer_data on MyType { ... MyChildContainer_data }`);

It should work because the fragment specified in the graphql text requests the data from the child container.

We are currently waiting on this PR to hit stable TS (should be TS 2.8): https://github.com/Microsoft/TypeScript/pull/21316

[reem]

Just encountered this myself and want to make sure I've correctly diagnosed the issue.

I have two fragments like so:

fragment list_resources on Resource @relay(plural: true) {
  ...list_resource
}

fragment list_resource on Resource {
  id
  other
  fields
}

and the type generated for list_resources is just ReadonlyArray<{}>. Can someone confirm this in fact due to this issue and should be fixed at some point in the relatively near future?

[kastermester]

@reem This is related yes. I have plans to fix this - but as it always happens, I'm quite busy.

Also please note that it is not that your generated type is wrong and will suddenly become: ReadonlyArray<{ id: ...; other: ...; fields: ... }> - the type is as intended in this case. This happens because relay is doing data masking. The subject at hand within this issue is to make sure that a component using the list_resources fragment can render another component using the list_resource component - using all the individual elements as props.

It's all a bit convoluted - let me know if it makes sense.

[reem]

My problem is that when I try to pass down an entry from the array to my child which expects a list_resource, I get a type error that {} does not match list_resource. Am I just annotating my types incorrectly? This is what I did to actually get it working:

//                                     this type wants to be just list_resources
class ResourceList extends React.Component<{ resources: list_resource[] }> {
   public render() {
     return this.props.resources.map((resource, ix) => <Resource key={ix} resource={resource}/>)
   }
}

class Resource extends React.Component<{ resource: list_resource }> { ... }

When I change list_resource[] to list_resources I get the type error I noted earlier. Is there something else wrong with what I am dong here? From what I understand this should be permitted by data masking because Resource is also a fragment container for the list_resource fragment.

EDIT: I guess I don't really understand the mechanism data masking actually uses to determine if data should be made available or not and I haven't been able to find any docs that actually explain things with technical detail and many examples. Do you have any resources handy?

[orta]

I don't know any in depth docs, but I can guide you through a trivial example on an app I'm working on which uses this lib. I think this might be what you're looking for.

I have an admin dashboard, I want to show the a collection of "installations", the full GQL query looks like this:

{
  me {
    name

    installationsToSetUp {
      edges {
        node {
          iID
          login
          avatarURL
        }
      }
    }
  }
}

I want to split it among components, so we start with the root query renderer here. The root query has access to the name on me. Then there's the fragment ...InstallationsToSetUp_user.

This component should not have access to things that are inside ...InstallationsToSetUp_user because it didn't ask for it. You can see that in the types that this lib generates:

export type dashboard_indexQueryResponse = {
    readonly me: ({
        readonly name: string;
    }) | null;
};

Yet, that data does have to be passed down the tree somehow to the component that owns the fragment InstallationsToSetUp_user. So, right now, because the repo isn't set up to let you pass the data in a way that is fully typed. Here's why:

The props for the InstallationsToSetUp require all the data from the full fragment for that component:

   fragment InstallationsToSetUp_user on User {
      installationsToSetUp {
        edges {
          node {
            iID
            login
          }
        }
      }
    }

As me in the root component only has a name, it can't be passed into something that it expects will have installationsToSetUp and all the other details. Statically, to the Type system anyway. However at runtime the props are a bit more complex than is actually exposed, and at runtime that is fixed.

So the long story short is, you can use as any to just let TS know that you know best, and use this<InstallationsToSetUp user={this.props.me as any} /> knowing fully well that the Relay runtime will actually fill it with the right data for that component.

---

I'm pretty sure this issue is about not having to eventually do the as any dance at this point, but I've not dug into it in depth

[orta]

WRT understanding how and why it works, I'd recommend grabbing the relay source code using the debugger and reading some of the stack traces to understand how these all come together - @alloy's LHOSST series shows you how to do a lot of that to grok complex codebases pretty quickly ( and concisely, if you have target)

[reem]

I see.. doesn't seem like a super easy fix, certainly having as any in my codebase is a big no-no. The whole magic and promise of integrating relay with typescript here is that typescript can be used to check all of your queries, fragments, and usage of data from them against your schema. Needing to cast things because the types don't tell the whole story is definitely unfortunate.

EDIT: Basically the current system encourages me to not compose fragments, or to manually use the lower fragments types in the parent component in order to get proper typechecking. Right?

EDIT2: Also just because I see you are using logical fragment names, how did you get the relay compiler to stop yelling at you for using names not derived from the module name? It's super annoying to name all my fragments the terrible names it generates haha.

[orta]

I'm not sure you grok the value of how data masking works based on these question. Each component only gets props what it requests, so it should not have the types for its children. As a parent props interface won't include the children then they can never match according to typescript. As we know this is fixed at runtime, you use as any. The child treats the props according to the interfaces generated and everything works. I think it's fine.

You can hack it by just including everything in the root, but you'd have to have the same query in all components, but that undermines the point and you'd be better off with apollo then for simplicities sake.

WRT filenames, I check the docs on the fragment container, you can name things whatever you want if you understand the constraints

[kastermester]

To sum up:

1. We could simply include all of the child properties in the parent fragment type, but this would be wrong. The parent container, at runtime, cannot access the child's properties.
2. We need a mechanism (this is what this issue is all about) to let TypeScript know that the parent's value is valid for the child component.

There's currently two workarounds:

1. Typecast your props with as any as @orta suggested.
2. When defining your fragments use the @relay directive to unmask the data (note: this is not recommended practice in Relay):

fragment list_resources on Resource @relay(plural: true) {
  ...list_resource @relay(mask: false)
}

This gives you the runtime semantics where the parent container can access the properties referenced by the child container (both in the types and at runtime). Like I said, I would strongly advice against this, as it breaks some of the core principles of using Relay. Instead the as any approach should be used until we finalize the support for fragment references, which fixes this - and should also make some other things work much better.

Note: The fix that is currently planned will require TypeScript 2.8 or newer.

[reem]

It makes sense that the type would not contain the full child fragment type due to data masking, but it is imo very important that it is possible to pass fragment props to children without needing an as any cast, doing as any basically throws out all the advantages of using typescript, it can no longer catch my mistakes. It sounds like we are on the same page, I look forward to using the final fix for this!

[alloy]

It sounds like we are on the same page, I look forward to using the final fix for this!

Yup 👍

[reem]

Are there any specific PRs I should be watching to this or other repos to know when the final fix will be available?

[alloy]

This ticket is to track that effort, any forthcoming PR should be linked here too.

[alloy]

@kastermester Reading up on your suggestion to use symbol rather than enum, we’d have to use unique symbol as per the documentation:

const _MySymbol: unique symbol = Symbol()
const _MyOtherSymbol: unique symbol = Symbol()

type MySymbol = typeof _MySymbol;
type MyOtherSymbol = typeof _MyOtherSymbol;

function x(sym: MySymbol): string { return 'hello'; }

declare const y: MyOtherSymbol;

x(y); // Should error here

It’s slightly unfortunate that there’s still a runtime cost for this, as there is with enum. The only option that would not have any runtime cost would be const enum, except that’s currently unsupported by Babel’s TS parser.

[alloy]

@kastermester Thinking about it some more, if we were to keep using enums then we could at least have an option to use const enums for optimisations. What do you think? 🤔

[kastermester]

@alloy You can simply use declare const _MySombol: unique symbol this should not (as far as I know) emit any code when compiling down to JavaScript.

Doing some testing I found that properties that are symbol indexed still show up under auto completion, so I think our way of prefixing the property with a space is the better idea here.

So to sum up, in the *.graphl.ts file we emit I suggest doing something like this:

import { MyOtherFragment$ref } from './MyOtherFragment.graphql';
export declare const MyFragment$ref: unique symbol;

export interface MyFragment {
  ' $refType': MyFragment$ref;
  ' $fragmentRefs': MyOtherFragment$ref;
  ....
}

Does this make sense @alloy ?

[alloy]

Interesting, I’ll give that a try 👍