Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

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

Open
fkling opened this issue Jul 18, 2016 · 7 comments

Comments

@fkling
Copy link

fkling commented Jul 18, 2016

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
Copy link
Contributor

bhosmer commented Jul 18, 2016

@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
Copy link
Contributor

bhosmer commented Jul 19, 2016

@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
Copy link
Author

fkling commented Jul 19, 2016

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
Copy link
Contributor

bhosmer commented Jul 19, 2016

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 added a commit to fkling/recast that referenced this issue Jul 20, 2016
... to include `TypeParameter` information (d5ce0400511bdf6b063c13728c4fa09edd9e122d). See also flow/flow-codemod#6 (comment) .
@fkling
Copy link
Author

fkling commented Jul 20, 2016

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 (benjamn/recast#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.

fkling added a commit to fkling/recast that referenced this issue Jul 20, 2016
... to include `TypeParameter` information (d5ce0400511bdf6b063c13728c4fa09edd9e122d). See also flow/flow-codemod#6 (comment) .
fkling added a commit to fkling/recast that referenced this issue Jul 20, 2016
... to include `TypeParameter` information (d5ce0400511bdf6b063c13728c4fa09edd9e122d). See also flow/flow-codemod#6 (comment) .
@bhosmer
Copy link
Contributor

bhosmer commented Jul 20, 2016

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
Copy link
Author

fkling commented Oct 5, 2016

Just FYI, using flow should work fine by now 😃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants