Definitelytyped: @types/react-relay (and friends): Some questions regarding possible upcoming pull request

Created on 9 Jan 2018  Ā·  15Comments  Ā·  Source: DefinitelyTyped/DefinitelyTyped

I'm working on trying to get a nice story between integration Relay (https://facebook.github.io/relay/) and TypeScript.

Currently there are definitions up for Relay 1.3.x - with the current version being 1.4.1. I'm not sure whether or not definitions for the new relay versions are being worked on or not.

I noticed when looking through the types for, especially, functions like createFragmentContainer that the function type does not represent the semantics of those functions. A function like createFragmentContainer has loosely speaking the following semantics:

  • It takes a React.ComponentType<Props> (+ fragment spec and more) and turns that into a React.ComponentType<NewProps> where:

    • NewProps have all the same props as Props (except any possible relay prop should be removed)

    • Any props that originates from relay fragment specified needs to be "transformed" into a different type that represents that the containers fragment should be fetched in the data that is being passed.

    • It should add possibility of using componentRef as a prop.

If one takes a look at the flow types from the react-relay package they handle it like so (from: https://github.com/facebook/relay/blob/master/packages/react-relay/modern/ReactRelayTypes.js) (the handling of componentRef is in the definition of createFragmentContainer):

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

In the flow types (and the types generated by relay-compiler) $refType on a fragment is set to a unique type for the given fragment and any object with __fragments property has that property set to an intersection type of all the fragment that it has spread out, this means that:

const x = graphql`fragment MyFragment on MyType { id ... MyOtherFragment ... MyOther2FragmentRef }`

Gets a type signature roughly equal to:

export enum MyFragmentRef {}
import { MyOtherFragmentRef } from '../MyOtherFragment.graphql';
import { MyOther2FragmentRef } from '../MyOther2Fragment.graphql';
export type MyFragment {
  $refType: MyFragmentRef;
  __fragments: MyOtherFragmentRef & MyOther2FragmentRef;
  id: string;
}

It is worth noting here that the $ObjMap type in flow has no direct translation to TypeScript, so doing it like this is not immediately possible.

However using some of the helper types referenced from Microsoft/TypeScript#12424 one should be able to replicate this behavior. See this playground.

These types should make it possible to create a similiar effect as to the flow types.

With the version of relay compiler I'm trying to create I intend to make the same kind of semantics in the type generation as the official relay compiler, only I plan to change the naming convention a bit (to take advantage of the fact that, in TypeScript, property names with spaces are excluded form the auto-completion). Thus I intend to rename $refType to $refType and __fragments to $fragments (or some other name with spaces in them). This is due to the fact that the types are largely unusable and not intended to be used by code written by hand.

My question is now this:

Is changing the types as mentioned here (using those "hacky" helper types) something that has potential to be accepted if a PR were to come? As far as I can tell those definitions would at the very least be slightly better than what is currently there (it will remove the relay prop type and add the componentRef type) - even if used by someone that does not use something to auto generate types for the fragments.

I also intend to initiate some dialog with the facebook team asking them whether or not they'd like my work on the relay-compiler to end upstream in relay-compiler - or if it should remain an external package with it's own version of the relay-compiler binary.

  • [x] [Mention](https://github.com/blog/821-mention-somebody-they-re-notified) the authors (see Definitions by: in index.d.ts) so they can respond.

    • Authors: @alloy, @voxmatt

Most helpful comment

@kastermester I actually already have a mostly finished version of relay-compiler that emits TS types, I only haven’t PR’ed it yet because the Relay team wants a plugin system and so I’m still working on that atm.

Here’s my uncleaned branch https://github.com/facebook/relay/compare/master...alloy:emit-typescript-types and here’s an example of a generated artefact: https://gist.github.com/alloy/724e156be0d5f745b33753492bf36145.

I did address:

  • Replace data props provided by Relay from the props of the composed fragment component by opaque props that only require a __fragments key to exist in the provided prop
  • Remove this.props.relay from the props of the composed fragment component
  • Some changes to the DT Relay typings to use the emitted code

I was, however, not able to figure out a way yet to make the opaque fragment props be of a more specific type than just ā€˜opaque fragment reference’, so if you have figured that out, that’s awesome!

How about I get my branch into a reviewable state by next week (currently on a deadline to release a new version of our app) and then you can go over it to make any further adjustments and especially your work to make proper use of the refType fragment references?

All 15 comments

@kastermester I actually already have a mostly finished version of relay-compiler that emits TS types, I only haven’t PR’ed it yet because the Relay team wants a plugin system and so I’m still working on that atm.

Here’s my uncleaned branch https://github.com/facebook/relay/compare/master...alloy:emit-typescript-types and here’s an example of a generated artefact: https://gist.github.com/alloy/724e156be0d5f745b33753492bf36145.

I did address:

  • Replace data props provided by Relay from the props of the composed fragment component by opaque props that only require a __fragments key to exist in the provided prop
  • Remove this.props.relay from the props of the composed fragment component
  • Some changes to the DT Relay typings to use the emitted code

I was, however, not able to figure out a way yet to make the opaque fragment props be of a more specific type than just ā€˜opaque fragment reference’, so if you have figured that out, that’s awesome!

How about I get my branch into a reviewable state by next week (currently on a deadline to release a new version of our app) and then you can go over it to make any further adjustments and especially your work to make proper use of the refType fragment references?

That sounds good! It would be nice if we could work on this together.

I see your approach is quite different from mine (regarding the compiler). I have (for now) been using the TypeScript APIs to both generate the TypeScript code and parse the code, figuring it was a more robust approach (mind you I only quickly glanced at your implementation).

The easiest way to create a unique type is simply to create an empty enum I have found. We have actually been using this in a similiar system on Relay Classic (it was tracking types, not fragment spreads though).

If you want I can grant you access to the private repo where I'm currently messing around on the Relay Compiler - maybe there's something of interest there for you to look at? The best way forward here, I guess, is some kind of merging between both of our approaches?

Great to see others working on this as well! :) I guess I was a bit silly assuming no one was doing this already.

I forgot to ask - have you looked into transforming the TS code for the Relay runtime?

I see there's a project like this: https://github.com/mattias800/relay-modern-typescript-transformer

But there's a few issues here:

  1. It expects a CommonJS export, which both you and I have gone away from in the compiler, due to mixed exports when also exporting types.
  2. It doesn't handle the classic Relay.QL transforms.

I was thinking perhaps I should look into creating a transformer that works with Relay modern for both Modern and Compat environments - to make sure we create a smooth experience when using Relay with TypeScript?

I know how it is to be busy with deadlines - I'm sure it will hit me at some point as well, so figured I might as well get going with something productive, while I have time.

I'm looking forward to working with you to get the proper experience when using TypeScript with Relay! :)

Heyyy! Yeah, busy busy busy :)

I see your approach is quite different from mine (regarding the compiler). I have (for now) been using the TypeScript APIs to both generate the TypeScript code and parse the code, figuring it was a more robust approach.

Seeing as the Relay team wants this to work through plugins, I’m actually coming around to just using the TS API like you are doing. The main reason I chose Babel 7’s ability to parse/generate TS code was that I was making the patch to be included _inside_ the relay-compiler package and so less dependencies would probably have been preferable.

The second reason, which still somewhat stands, but maybe we can make that work while still using the TS API, is that in general the type generation code for both Flow and TS would be largely the same, so I wanted to leverage the existing code (also for future ease of maintenance). Not sure yet how I feel about that now, what do you think?

have you looked into transforming the TS code for the Relay runtime?

I see there's a project like this: https://github.com/mattias800/relay-modern-typescript-transformer

But there's a few issues here:

  1. It expects a CommonJS export, which both you and I have gone away from in the compiler, due to mixed exports when also exporting types.

  2. It doesn't handle the classic Relay.QL transforms.

I was thinking perhaps I should look into creating a transformer that works with Relay modern for both Modern and Compat environments - to make sure we create a smooth experience when using Relay with TypeScript?

Yeah my plugin design allows the plugin to do the parsing of the input code, so no need for a separate transformer.

I hadn’t looked into the classic stuff yet, mostly because I wasn’t sure if I actually needed to spend time on that. The reasoning being, people that have classic code, like we had too, were already able to cope just fine. If they want the benefit of the plugin, then they should just migrate to Modern. Do you have any arguments to the contrary?

If you want I can grant you access to the private repo where I'm currently messing around on the Relay Compiler - maybe there's something of interest there for you to look at? The best way forward here, I guess, is some kind of merging between both of our approaches?

Yup, that sounds good!

What I think we should be doing to get this merged and working easily is:

  • I finish a rough PR for relay-compiler that adds a simple plugin API (which is both for input parsing and artefacts output)
  • we make your TS API using code into a standalone plugin package
  • we update the DT typings to make the best use of the artefacts

PS: I can do that rough plugin PR very soon, but you can start the move to make your code into a standalone package today, if you want. In essence the plugin needs to:

  1. define a function that given a pathname transpiles source to ES6 JS, relay-compiler can take over from there and parse out the fragments
  2. define a function that takes GraphQL AST and generates type information for it
  3. define a function that stores the artefact file given a pathname, the previously generated type information, and the data for relay-runtime to use

We can see if we can make 1 more efficient by not leaving parsing out the fragments to relay-compiler (because it requires TS -> JS -> Babel -> JS) but as a first step I’d say let’s keep that simpler.

I will give you access to the repo right away.

  1. There's code in there to parse TS code and return a GraphQL document containing all the relevant GraphQL code in there (just like the RelayJSModuleParser does). It is actually mostly quite simple and not much more complicated than transpiling the code to JS. The main part here is that there's some code inside relay to validate the names of the fragments - which sort of needs to be copy/pasted. However simply exporting these functions from the relay package should sort out quite a bit of the needed code there.

  2. When you mean GraphQL AST you mean the GraphQLIR from the relay-compiler package I assume? I have code for that in the repo as well.

  3. If I understand you correct here you basicly want a specific implementation of the writeRelayGeneratedFile module. I created a PR to allow configuring the file extension and then were able to reuse the existing module inside Relay for this - only adding a module formatter. The PR is here: https://github.com/facebook/relay/pull/2263

Note: My repo is based off of the PR I just linked, thus you need to compile relay-compiler and relay-runtime from that repo and npm pack them and then install them to use the code. Also I have currently just focused on getting the code to run, my plan was to port most, if not all, of the related tests from the relay-compiler package itself. Also I'm not sure if writing the plugin in TS or Flow is most beneficial, either way it seems to require some type definitions to be written to be best suited (either for the typescript package if written in flow, or the relay-compiler package if written in typescript).

The idea around a plugin making the work sounds good! It also seems to be the most flexible way going forward - if there's other programming languages that need similiar hooks.

  1. When you mean GraphQL AST you mean the GraphQLIR from the relay-compiler package I assume? I have code for that in the repo as well.

Correct šŸ‘

  1. If I understand you correct here you basicly want a specific implementation of the writeRelayGeneratedFile module. I created a PR to allow configuring the file extension and then were able to reuse the existing module inside Relay for this - only adding a module formatter. The PR is here: facebook/relay#2263

Yup, but there’s more to it than just the extension. I would suggest you close that PR with a link to this discussion. The Relay team already knows I’m working on the same things, so updating them and all other readers that we are going to undertake a more holistic approach would probably alleviate pressure from the Relay team to have to take time to comment on this when we already know the approach we need to take.

Also I'm not sure if writing the plugin in TS or Flow is most beneficial, either way it seems to require some type definitions to be written to be best suited (either for the typescript package if written in flow, or the relay-compiler package if written in typescript).

I think we should (alas) use Flow, because we’ll indeed be importing stuff from relay-compiler, including e.g. a Flow plugin interface that we should conform to.

Exciting, I’m eagerly looking forward to cleaning up this plugin stuff and get all our work into a mergeable state! Let’s continue discussion on the repo you invited me to.

Sounds good. I will open some issues regarding some of the things we have talked about here so we can discuss them in a more organized manner there. I think writing it in flow sounds reasonable. I ended up typing up quite a bit of the relay-compiler package (as you can see in the repo) and I think maintaining that over time will get very cumbersome. I expect the TypeScript APIs to be a bit more stable, and also it is probably a smaller API surface there we need to touch.

Oh, actually i just remembered that there’s no Flow included in the distribution packages iirc, so the plugin wouldn’t actually be able to make use of Flow types :/ We should check if this is really the case and if so maybe we can leave it as TS for now?

Perhaps, I also just realised that perhaps the best way to handle this (if there are flow types exported, or if we can get the Relay team to export them) would be to create a mixed repository. Ie. we could easily create the code to find graphql tagged template literals and return DocumentNodes in TS and call it from flow.

But let's find out what seems best here. All in all I don't really care which language we write it in, as long as we make things as easy for ourselves as we possibly can.

Hi. Any updates on this?

I think we can recreate $RelayProps using mapped types and conditions types

Oh my, didn’t realize this was still open. I think it can be closed now, you should use Relay >= 1.7.0 and our TS plugin https://github.com/relay-tools/relay-compiler-language-typescript combined with the latest Relay related typings from this repo.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jbreckmckye picture jbreckmckye  Ā·  3Comments

jamespero picture jamespero  Ā·  3Comments

victor-guoyu picture victor-guoyu  Ā·  3Comments

csharpner picture csharpner  Ā·  3Comments

Zzzen picture Zzzen  Ā·  3Comments