Generate GQL fragments based on TS types #14

RylanH · 2017-08-26T20:33:41Z

Often times we find ourselves having to keep the GQL fragments and the data we expect in an application in sync. Instead, this PR enables generation of a GQL fragment from TypeScript typings. This is great because we oftentimes already have typings such as React Component typings for props or on the server side typings we apply to the data we received. By generating the fragment and writing it to a file, it makes the fragment inspectable as well for easier debugging.

Here's how it works:

Suppose I have a TS program that looks like this:

import { fragment } from 'ts2gql';
import gql from 'graphql-tag';

interface PostContent {
  title:string;
  body:string;
}

type PostTitle = Pick<PostContent, 'title'>;

const query = gql`
  query getTitles() {
    posts() {
      ...${fragment<PostTitle, PostContent>('./getTitles.graphql')}
    }
  }
`;

This will automatically generate getTitles.graphql with a fragment that looks like this:

fragment getTitles on PostContent {
  title
}

Which using the graphql loader will get picked up at runtime and used.

yuit

I think we should add test for with nested interface type and primitives?

yuit · 2017-09-23T00:35:25Z

src/Fragment.ts

+
+function getFragmentDeclaration(files:typescript.SourceFile[]) {
+  let fragmentDeclaration:typescript.FunctionDeclaration|null = null;
+  const thisTypeFile = files.find(f => f.fileName === `${__filename.substr(0, __filename.length - 3)}.d.ts`);


nit: could you add comment about the substr operation? what is the input filename and expected output?

yuit · 2017-09-23T00:48:52Z

src/Fragment.ts

+  // Get the declaration of the fragment function contained in this file
+  const fragmentDeclaration = getFragmentDeclaration(files);
+
+  if (!fragmentDeclaration) {


Q: should we try to reconcile two error throwing here in the case of you don't have a fragment import, you will get two errrors

How so? Won't it throw once and bubble all the way up?

yep you are right

yuit · 2017-09-23T00:53:08Z

src/Fragment.ts

+      calls = calls.concat(childCalls);
+    }
+    if (child.kind !== typescript.SyntaxKind.CallExpression) return null;
+    const call = child as typescript.CallExpression;


nit: use type annotation instead of casting

If I do, it says

[ts] Type 'Node' is not assignable to type 'CallExpression'. [ts] Type 'Node' is not assignable to type 'CallExpression'. Types of property 'kind' are incompatible. Type 'SyntaxKind' is not assignable to type 'SyntaxKind.CallExpression'.

yuit · 2017-09-23T00:54:07Z

src/Fragment.ts

+
+    const type = checker.getTypeOfSymbolAtLocation(symbol, call.expression);
+
+    if (!type.symbol || type.symbol.valueDeclaration !== fragmentDeclaration) return null;


nit: add comment about why checking type.symbol.valueDeclaration !== fragmentDeclaration?

yuit · 2017-09-23T00:55:32Z

src/Fragment.ts

+    }
+
+    const data = call.typeArguments[0];
+    if (data.kind !== typescript.SyntaxKind.TypeReference) {


nit: this check and below check should merge (https://github.com/convoyinc/ts2gql/pull/14/files#diff-7a0c154a11c76dcb49ac0bf9d958a9a9R106)

Why? Don't we want to check each argument?

yep you are right... they do report different errors 👍

yuit · 2017-09-23T01:01:36Z

src/Fragment.ts

+      throw new Error('ts2gql.fragment<TFragment, TFragmentBase>(require(relGQLPath)): TFragmentBase must be a TypeReference');
+    }
+    const gqlToken = call.arguments[0];
+    const relativePath = (gqlToken as typescript.StringLiteral).text;


Q: should we check that gqlToken is a string literal?

yuit · 2017-09-23T01:03:13Z

src/Fragment.ts

+  const fields:Field[] = [];
+
+  // For Arrays we want to use the type of the array
+  if (type.symbol && type.symbol.name === 'Array') {


Q: should we add test for this case?

yuit · 2017-09-23T01:05:38Z

src/Fragment.ts

+  }
+
+  // For unstructured types (like string, number, etc) we don't need to loop through their properties
+  if (!(type.flags & typescript.TypeFlags.StructuredType)) return null;


Should we check for TypeFlags.Primtive (https://github.com/Microsoft/TypeScript/blob/master/src/compiler/types.ts#L3230) I think we don't want to loop through primitive type in general

…into rylanh/fragments

yuit

just couple stuffs

yuit · 2017-12-19T17:29:36Z

src/Emitter.ts

@@ -8,7 +8,7 @@ export default class Emitter {
  renames:{[key:string]:string} = {};

  constructor(private types:Types.TypeMap) {
-    this.types = <Types.TypeMap>_.omitBy(types, (node, name) => this._preprocessNode(node, name!));
+    this.types = <Types.TypeMap>_.omitBy(types, (node, name) => this._preprocessNode(node, name!, types));


nit: omitBy is a generic function so instead of casting I think it is better to do _.omitBy<Types.TypeMap>.

yuit · 2017-12-19T17:33:31Z

src/Emitter.ts

@@ -211,8 +211,8 @@ export default class Emitter {
  }

  _indent(content:string|string[]):string {
-    if (!_.isArray(content)) content = content.split('\n');
-    return content.map(s => `  ${s}`).join('\n');
+    if (!_.isArray(content)) content = (content as string).split('\n');


Q: Why do just do Array.isArray(content) also since you are doing type guard here I don't think you need casting

yuit · 2017-12-19T17:36:11Z

src/Fragment.ts

+  // Get the declaration of the fragment function contained in this file
+  const fragmentDeclaration = getFragmentDeclaration(files);
+
+  if (!fragmentDeclaration) {


yep you are right

yuit · 2017-12-19T17:41:20Z

src/Fragment.ts

+      const fileName = path.basename(gqlPath, path.extname(gqlPath));
+      mkdirp.sync(path.dirname(gqlPath));
+
+      let contents = '';


use template string

// it will have new line added const contents = `fragment ${fileName} on ${call.baseName} { emitFields(call.properties)} `;

yuit · 2017-12-19T17:43:04Z

src/Fragment.ts

+    }
+
+    const data = call.typeArguments[0];
+    if (data.kind !== typescript.SyntaxKind.TypeReference) {


yep you are right... they do report different errors 👍

yuit · 2017-12-19T17:58:49Z

src/Fragment.ts

+
+export function fragment<TFragment extends Partial<TFragmentBase>, TFragmentBase>(filepath:string) {
+  // Some pointless code to appease error TS6133: 'TFragment' is declared but its value is never read.
+  const ignore:TFragment|null = null;


This is a bit weird but i don't know how to make it better either

RylanH added 4 commits May 29, 2017 23:08

Generate GQL fragment based on types

a061673

Merge branch 'master' into rylanh/fragments

7cdca72

Update dependencies, fixup some references/typings, write to files

c1fa86d

Remove TODO

27fb01f

RylanH requested a review from nevir August 26, 2017 20:35

Rylan Hawkins and others added 4 commits August 28, 2017 17:35

Using the actual symbol

a7c1dbf

Cleanup type system, make directory p, use correct name

59164d0

Fixup ts2gql

6e6f9c2

Fixing some TS 2.4 issues

fd2453c

RylanH requested a review from yuit September 9, 2017 05:18

RylanH added 2 commits September 10, 2017 14:22

Some fixes when running against more production scenarios

81322ac

Fixing up arrays

c4fc4b3

yuit reviewed Sep 23, 2017

View reviewed changes

RylanH and others added 7 commits November 7, 2017 09:09

Adding some comments

499ba9c

Merge branch 'master' into rylanh/fragments

e76c118

Merge branch 'rylanh/fragments' of https://github.com/convoyinc/ts2gql …

2610685

…into rylanh/fragments

Write first test and get it running

793be06

Reorganize files and test

05110ea

Fixing circle

5fc0550

Undo bin directory references

4fe9a74

yuit approved these changes Dec 19, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate GQL fragments based on TS types #14

Generate GQL fragments based on TS types #14

RylanH commented Aug 26, 2017 •

edited by yuit

Loading

yuit left a comment

yuit Sep 23, 2017

yuit Sep 23, 2017

RylanH Nov 7, 2017

yuit Dec 19, 2017

yuit Sep 23, 2017

RylanH Nov 7, 2017

yuit Sep 23, 2017

yuit Sep 23, 2017

RylanH Nov 7, 2017

yuit Dec 19, 2017

yuit Sep 23, 2017

yuit Sep 23, 2017

yuit Sep 23, 2017

yuit left a comment

yuit Dec 19, 2017

yuit Dec 19, 2017

yuit Dec 19, 2017

yuit Dec 19, 2017

yuit Dec 19, 2017

yuit Dec 19, 2017


		const type = checker.getTypeOfSymbolAtLocation(symbol, call.expression);

		if (!type.symbol \|\| type.symbol.valueDeclaration !== fragmentDeclaration) return null;

Generate GQL fragments based on TS types #14

Are you sure you want to change the base?

Generate GQL fragments based on TS types #14

Conversation

RylanH commented Aug 26, 2017 • edited by yuit Loading

yuit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RylanH commented Aug 26, 2017 •

edited by yuit

Loading