Fix GitHub syntax highlighting

[calebmer]

I often pull up the src/language/ast.js file as a reference when working with a GraphQL query. However, the syntax highlighting on GitHub is broken. This PR should maintain the same behavior while fixing the syntax highlighting on GitHub with the side benefit of exporting TokenKind separate of Token.

A before photo is attached below.

• Before:

  graphql-js/src/language/ast.js

  Line 76 in 2235563

  /**

• After: https://github.com/calebmer/graphql-js/blob/da09c4ef1e5dd5ae2d99a73296a3274a03b609c7/src/language/ast.js#L84

[asiandrummer]

graphql-js/src/language/lexer.js

Lines 84 to 131 in da09c4e

	// Each kind of token.
	const SOF = '<SOF>';
	const EOF = '<EOF>';
	const BANG = '!';
	const DOLLAR = '$';
	const PAREN_L = '(';
	const PAREN_R = ')';
	const SPREAD = '...';
	const COLON = ':';
	const EQUALS = '=';
	const AT = '@';
	const BRACKET_L = '[';
	const BRACKET_R = ']';
	const BRACE_L = '{';
	const PIPE = '|';
	const BRACE_R = '}';
	const NAME = 'Name';
	const INT = 'Int';
	const FLOAT = 'Float';
	const STRING = 'String';
	const COMMENT = 'Comment';
	/**
	* An exported enum describing the different kinds of tokens that the
	* lexer emits.
	*/
	export const TokenKind = {
	SOF,
	EOF,
	BANG,
	DOLLAR,
	PAREN_L,
	PAREN_R,
	SPREAD,
	COLON,
	EQUALS,
	AT,
	BRACKET_L,
	BRACKET_R,
	BRACE_L,
	PIPE,
	BRACE_R,
	NAME,
	INT,
	FLOAT,
	STRING,
	COMMENT
	};

already defines this - maybe we should just reuse?

[calebmer]

Didn’t know that existed. At a minimum we shouldn’t export TokenKind then.

Does Flow have a way to get the type of all the values? I know that TypeScript has keyof which will union all the string literal key types. Something like $PropertyType<T, x>, but it returns a union of all the property types and not just one?

Assuming the property types are all string literals we could then write:

type Token = {
  kind: $PropertyTypes<TokenKind>,
  ...
};

This would also be a reverse $Keys<T>.

[asiandrummer]

I'm not sure if there's a support for that yet - we probably need to do in a verbose way for the time being.
Another thing I noticed is that ast.js uses Token object solely to typecheck Location object - we could potentially move export type Token to lexer.js and import the type from there.

[calebmer]

I like that better. I moved the Token type as it is on master into the lexer.js file.

[wincent]

Thanks for this @calebmer. I've also found the broken highlighting in GitHub to be bothersome. Travis is complaining about a failure under Node 7 though, with a legit looking error message, although I am not sure why it would only happen in version 7.

[calebmer]

@wincent fixed. It was indeed a legitimate Flow error. It looks like Flow only runs on one of the Travis jobs:

graphql-js/.travis.yml

Line 29 in 1eb41dd

- if [[ "$TRAVIS_JOB_NUMBER" == *.1 ]]; then npm run lint && npm run check && npm run cover:lcov; else npm run testonly; fi

, so it would make sense that it only fails on Node.js 7. That’s smart 😊

[wincent]

So this doesn't appear to fix the GitHub syntax highlighting, it merely moves the brokenness into a different file. I think this will work instead (at least, it works when I tested it in a Gist):

diff --git a/src/language/ast.js b/src/language/ast.js
index 3639ec9..001900d 100644
--- a/src/language/ast.js
+++ b/src/language/ast.js
@@ -44,15 +44,11 @@ export type Location = {
 };
 
 /**
- * Represents a range of characters represented by a lexical token
- * within a Source.
+ * Token kinds, defined at the top level to avoid broken syntax
+ * highlighting on GitHub.
  */
-export type Token = {
-
-  /**
-   * The kind of Token.
-   */
-  kind: '<SOF>'
+type TokenKind
+      = '<SOF>'
       | '<EOF>'
       | '!'
       | '$'
@@ -73,6 +69,17 @@ export type Token = {
       | 'String'
       | 'Comment';
 
+/**
+ * Represents a range of characters represented by a lexical token
+ * within a Source.
+ */
+export type Token = {
+
+  /**
+   * The kind of Token.
+   */
+  kind: Kind;
+
   /**
    * The character offset at which this Node begins.
    */

[calebmer]

@wincent this approach was roughly my first commit: da09c4e

Moved away from that because @asiandrummer mentioned the existing naming conflict with TokenKind in lexer.js and recommend we move the Token type into lexer.js, but keep the union inline. I like the idea of Token being in lexer.js instead of ast.js, but that was not the original point of the PR.

If you want we can revert back to da09c4e and then not export TokenKind for ast.js with a comment noting that the type alias is only to fix syntax highlighting.

[wincent]

Yeah, the diff was just an example to highlight what kind of change would be need to be made to fix the highlighting problem rather than just moving it.

If you want we can revert back to da09c4e and then not export TokenKind for ast.js

Yep, and you'll note that in my diff I didn't export it.

with a comment noting that the type alias is only to fix syntax highlighting.

Yep, that's what I did in my diff too. I'd be happy with that or an alternative that moves stuff into lexer.js but also fixes the busted highlighting in that file. I'll leave it up to your judgment.

[calebmer]

Ok, reverted back to the original changes in order to keep this PR minimal and focused. As I mentioned before, the best solution would probably be to run a $PropertyTypes<T> utility type which would be a reverse of $Keys<T> (and a generalization of $PropertyType<T, x>) in Flow on the TokenKind object exported in lexer.js. However, such a utility type is not available, so this should be the simplest solution that removes the syntax highlighting problem 😊

Other options for the type name are TokenKindUnion or TokenKindLiteral if having two TokenKinds in the codebase is too confusing.

[wincent]

Thanks for this @calebmer. I think what you've done here strikes the right balance.