Significant enhancements

[benjie]

I'm very limited on time but I managed to make this basically work for the Graphile Engine codebase. No doubt there's going to be a bunch of cleanup, but I though I'd share my modifications in case they're helpful to others. Sorry about all the code formatting changes, this is not a genuine PR that I expect you to merge, just felt bad keeping my changes to myself. Feel free to close!

This PR includes a lot of improvements - more types are converted, existing supported types are converted more accurately leading to better formatting of outputted code, TypeScript definitions are more thorough.

[bcherny]

Hey @benjie! Thanks for the contribution. I have to say, without tests and with all the formatting changes (single quote -> double, semicolons), this PR is super hard to review.

Would it be too much to ask to revert your formatting changes, and add a few tests for your biggest changes?

[benjie]

If I have sufficient time left once I’ve completed the TypeScript conversion for PostGraphile I’ll certainly try.

Sorry I let my autoformatter go mad on the codebase, I wasn’t expecting to do more than a couple tweaks, ended up being much bigger changes.

More to come.

[benjie]

Upgraded to the latest versions of various dependencies so have managed to remove a bunch of @ts-ignore comments. Addressed a few more bits of feedback.

[benjie]

And another commit to parenthesise functions definitions in union types.

[DylanVann]

I believe it's specifically comments preceding/following nodes that are replaced by the converter.

The same problem can occur when writing codemods, as I'm experiencing right now trying to write a codemod to do some other transformations as part of this conversion.

@babel/preset-typescript doesn't like it when you do export { AType } because AType will be undefined, so I'm doing a pass of converting those to inline exports. If they're inline then the whole line goes away and Babel doesn't complain.

Might be something to include in this repo as well.

[benjie]

Added support for function rest parameters and use lazy heuristics to copy more comments across.

[benjie]

@bcherny This is actually a genuine PR now, and ready for review.

Here's the result of running it against the Graphile Engine codebase. I've not attempted to actually compile the generated TypeScript yet, but it looks broadly okay by eye.

https://github.com/graphile/graphile-engine/pull/370/files

[benjie]

Okay, now it's ready for review. I re-enabled noImplicitAny which means I've had to add a few explicit any. They're fine.

I've also pulled in more correct type files, so visitors are now type-checked too 🎉

[niieani]

Looks like there's an off-by-one error in how comments are handled in the example PR you mentioned: https://github.com/graphile/graphile-engine/pull/370/files.

What I mean is:

• the preceding comments are now inline: example
• the inline comments are now in the next line: example

[benjie]

@niieani Indeed; that's at least better than losing the comments though.

Here's the code that handles copying the comments across, if you can suggest improvements I'm all ears 👍

flow-to-typescript/src/convert.ts

Lines 224 to 234 in 04ccd25

	function copyCommentsToFrom(to: TSType | Node, from: FlowType | TSType | Node) {
	to.leadingComments = from.leadingComments
	to.innerComments = from.innerComments
	to.trailingComments = from.trailingComments
	}
	export const toTs: typeof _toTs = (node: any) => {
	const newNode = _toTs(node)
	copyCommentsToFrom(newNode, node)
	return newNode
	}

[DylanVann]

I like that it's more generalized, I fixed that issue on my fork, but it only works one level deep, which isn't very good.

[DylanVann]

I came up with a less drastic way of improving the tests, so I can keep track of changes more easily.

This will allow you to use AVA's -u or --update-snapshots CLI arguments to update the output.txt files.

Feel free to add to this PR or repo if you think it helps:

Code (since I may force push on my fork later)

import test from 'ava'
import { sync } from 'glob'
import { readFile, writeFile } from 'mz/fs'
import { basename, resolve } from 'path'
import { compile } from '../src'

let paths = ['e2e', 'rules', 'unit']

// Kind of hacky thing to figure out if -u or --update-snapshots was passed to AVA.
// We manually write expected output in this test.
// It's more clear, and allows us to have input and output files for each test.
const argvString = process.env.npm_config_argv as string
const argv = JSON.parse(argvString || '')
const argvOriginal = argv.original || []
const update =
  argvOriginal.includes('-u') || argvOriginal.includes('--update-snapshots')

paths.forEach(path => {
  // TODO: Why does glob catch tslint.json even with the trailing slash?
  const folders = sync(resolve(__dirname, `../../test/${path}/*/`))
    .filter(_ => !_.endsWith('.json'))
    .filter(_ => !basename(_).startsWith('_'))

  const tests = folders.map(folder => ({ folder, name: basename(folder) }))

  tests.forEach(({ name, folder }) =>
    test(name, async t => {
      try {
        const inputPath = resolve(folder, 'input.txt')
        const outputPath = resolve(folder, 'output.txt')
        const input = await readFile(inputPath, 'utf-8')
        const output = await compile(input, inputPath)
        if (update) {
          await writeFile(outputPath, output)
          t.pass()
        } else {
          const expectedOutput = await readFile(outputPath, 'utf-8')
          t.is(output, expectedOutput)
        }
      } catch (e) {
        console.log('error', e)
      }
    })
  )
})

[benjie]

This PR has diverged too far to be a sensible PR any more; so I've forked the project to benjie/flow2typescript and released it to npm.

[benjie]

0.2.0 of flow2typescript is out including a large number more fixes; we've used it to help translate GraphiQL to TypeScript and I'm currently using it to convert Graphile Engine.

[niieani]

Thanks for the update @benjie. I wonder if you saw or @bcherny saw two other projects that aim to convert flow to typescript that I have found out about in the last months:

• https://github.com/grubersjoe/reflow
• https://github.com/zxbodya/flowts

I haven't had the chance to compare them with your update, but they both seem to have had quite a bit of progress.

[benjie]

No, I've not researched those. They're welcome to merge in parts of my code if they want; it's all MIT licensed. (I'm not planning to work on this any more, flow2typescript does everything I need it to do.)

[joebowbeer]

Adding to list of alternates: https://github.com/Khan/flow-to-ts