flow / flow-codemod

Flow codemod scripts
MIT License
15 stars 3 forks source link

Update documentation to suggest the use of `--parser=flow` or `--parser=bablyon` #6

Open fkling opened 8 years ago

fkling commented 8 years ago

I noticed that you warn users that not every files may be parsed with Babel 5.

Since v0.3.21, jscodeshift allows to specify a different parser, including flow and babylon. I haven't done extensive testing yet, but if you find that either parser works for your use cases, updating the examples to include --parser=... could be useful.

bhosmer commented 8 years ago

@fkling Hi Felix - you're right, we should make that clear. I haven't had time to try the different parsers or update the examples, but in the meantime will update the readme to note their availability. Thanks!

bhosmer commented 8 years ago

@fkling Hi Felix - when I add --parser=flow to the commandline for strict-type-args transform, after rerunning npm install -g jscodeshift on my laptop, I get the following:

bhosmer$ jscodeshift -t transforms/strict-type-args/strict-type-args.js ~/test --errors=/Users/bhosmer/test/errs.json --parser=flow
Processing 1 files... 
Spawning 1 workers...
Sending 1 files to free worker...
worker: loaded 1 arity errors (of 2 total) from /Users/bhosmer/test/errs.json
 ERR /Users/bhosmer/test/test.js Transformation error
Error: did not recognize object of type "TypeParameter"
    at Object.getFieldNames (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/types.js:659:15)
    at visitChildren (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:221:32)
    at Visitor.PVp.visitWithoutReset (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:202:16)
    at NodePath.each (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path.js:99:22)
    at visitChildren (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:217:14)
    at Visitor.PVp.visitWithoutReset (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:202:16)
    at visitChildren (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:244:21)
    at Visitor.PVp.visitWithoutReset (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:202:16)
    at visitChildren (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:244:21)
    at Visitor.PVp.visitWithoutReset (/Users/bhosmer/.nvm/versions/node/v5.9.1/lib/node_modules/jscodeshift/node_modules/recast/node_modules/ast-types/lib/path-visitor.js:202:16)
All done. 

Any idea what might be going on? Happy to open an issue, but thought I'd check with you first in case it's something obvious I'm doing.

fkling commented 8 years ago

Great! (that you got a useful stack trace, not that you get an error :-/ ) I ran into this issue as well in astexplorer (when I used type T<A=number> = foo;), but didn't get a useful stack trace.

This looks like a problem with ast-types. It seems there is a TypeParameter node where it didn't expect one. Some definition in ast-types probably just needs to be updated.

If you can find out which exact declaration produces the error (maybe it's similar to mine?), we can look at the corresponding ast-types definition and see what needs to be updated.

bhosmer commented 8 years ago

Yeah, I think you're right that it's ast-types. Here's the file that I was parsing:

// @flow                                                                                                                                                                                       

type Foo<T> = { x: T }; 
//       ^ guessing the error was here, basically like yours

function f(x: Foo<any>) { return x }

I think you may just need to upgrade ast-types? But FYI just talked to @samwgoldman and there will probably be another change coming (type param variance annotations are currently just strings, but will become subnodes reused by property variances).

Noob question, but is there any way things could be set up to spare you the manual labor of checking and updating the flow parser's dependencies?

fkling commented 8 years ago

A fresh install of jscodeshift should always pick up the latest version of recast. Unfortunately, the current version of recast (v0.11.10) is locked to ast-types v0.8.17, which doesn't contain the TypeParameter declaration.

I created a PR (https://github.com/benjamn/recast/pull/304) to update recast to use ast-types v0.8.18. A fresh install of jscodeshift (and recast) should then fix the issue.

No need to make any changes to jscodeshift itself, but we can update the recast version in package.json too.

bhosmer commented 8 years ago

Ah, ok. I'll check back and bump the docs once recast is up to date and the workflow goes through - it'll be a big plus for flow-based codemods, that's for sure. Thanks again for adding the option!

fkling commented 7 years ago

Just FYI, using flow should work fine by now 😃