Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript support? #180

Closed
LudwikJaniuk opened this issue Jan 5, 2017 · 43 comments
Closed

Typescript support? #180

LudwikJaniuk opened this issue Jan 5, 2017 · 43 comments
Labels

Comments

@LudwikJaniuk
Copy link

This library is awesome, but I can't use it since my codebase is in typescript. Any chance of TS support in the future? Or could you maybe point me in the direction of similar projects that work for TS?

@fkling
Copy link
Contributor

fkling commented Jan 18, 2017

In theory it is possible to support any language. Things that would need to be done:

  • Create custom AST node definitions for recast/ast-types (if necessary)
  • Create custom pretty printer for recast or extend recast's existing printer
  • Create custom scope resolution logic for recast (if necessary, probably the most difficult part)
  • Create language specific jscodeshift API (if desired).

Not using TypeScript myself, I don't know of any similar tool for it.

@ianks
Copy link

ianks commented Jun 29, 2017

babel/babylon#523 is a big step towards making this happen. Perhaps this justifies reopening the issue?

@pbomb
Copy link

pbomb commented Jul 25, 2017

Has the landscape changed enough for this to be possible now? I know the Babylon parser has typescript support in the v7 beta, but not sure if the recast portion of the tooling has any support yet.

@skovhus
Copy link
Contributor

skovhus commented Aug 22, 2017

But be really nice to make this happen. @fkling can you reopen?

Also see benjamn/recast#424

@benjamn
Copy link

benjamn commented Aug 22, 2017

Echoing @fkling's comment above, I've outlined what's required on the Recast side here.

While I agree with @fkling about the steps, I'm generally pretty optimistic about this effort, and I would note that only the first two steps are critical to implementing TypeScript support in Recast (adding the types to ast-types and adding new printer cases to Recast).

Whether or not this happens ultimately depends on whether the work gets done (is that a tautology? maybe 😛 ). I'm happy to help, and I imagine @fkling is cautiously happy to provide some guidance, but I don't think either of us can commit to this project in a serious way unless we get a lot of help from the community.

@blaster151
Copy link

I am very interested in using jscodeshift on a TypeScript code base. I might have some learning curve on the parsing and AST stuff, but I'd be willing to help with this effort if I can!

@ianks
Copy link

ianks commented Sep 1, 2017

I used the flow parser on a typescript codebase and it actually worked very well. @blaster151 perhaps you could use it as a starting point?

@blaster151
Copy link

Hmm. Thanks, @ianks! I did not consider that and I will look into it!

@georgeedwards
Copy link

Same position as @blaster151 Happy to help, not so good on AST, but good on TypeScript, so if anyone can point mein the right direction, I'll make a start :)

@sjy
Copy link

sjy commented Dec 4, 2017

I made a repo containing some simple codemods to transform js{x} based react code to tsx based code. IMO, it is possible to parse react code using babylon parsor
https://github.com/sjy/js2tsx

@AmmarHasan
Copy link

I copied an Angular(typescript) component in astexplorer.net and a simple codemod that reverses identifiers (https://gist.github.com/de28761373f95a0615a4ef9bc4155822/e73fd0cdec045078763954f6d2f58a434ab8f429).

It works fine if the component does not contain "public" or "private" keyword but if it does following error is occurring:
"The keyword 'public' is reserved error"

@blaster151
Copy link

I can use the Flow parser to parse TypeScript with the exception of the casting operator (as in, myValue). (The alternate syntax ,"myValue as any", works fine - any ideas?)

@RupenAnjaria
Copy link

RupenAnjaria commented Feb 1, 2018

What is the concluding note here? Should we use Flow parser or parser created by @sjy? Further, can you please provide any example on how to use Flow parser for migration?

@elliottsj
Copy link
Contributor

elliottsj commented Feb 11, 2018

@Phoenixmatrix has been doing some great work to add TypeScript support to both ast-types and recast: benjamn/recast#424 (comment)

This paves the way for jscodeshift to support TypeScript transforms. I wrote a quick example here: https://github.com/elliottsj/jscodeshift-transforms

To make this work while those pull requests are pending, that repo has an npm dependency on
https://github.com/elliottsj/jscodeshift/tree/typescript, which has an npm dependency on
https://github.com/elliottsj/recast/tree/typescript-initial, which has an npm dependency on
https://github.com/Phoenixmatrix/ast-types/tree/typescript-initial

This is an example transform which works on TypeScript:
https://github.com/elliottsj/jscodeshift-transforms/blob/master/transforms/reverse-identifiers.js
The key is to have module.exports.parser defined in the transform module, which will parse code using babylon@7 (which can parse TypeScript AST nodes). At some point, it may be a good idea to update jscodeshift itself to use babylon@7 when the --parser babylon option is used, but I'm not sure whether that should wait until babylon@7 is out of beta.

@Phoenixmatrix
Copy link

@elliottsj btw, one thing I'm not sure how jscodeshift should handle: the difference between TS and TSX. In Babylon world, as far as I understand, the difference is just if you add the jsx plugin or not. But jscodeshift is usually used to process a lot of files, and it would make sense to be able to go through ts and tsx files in one run, thus requiring to toggle the parser based on file extension or something.

If your code never use <foo>bar style assertion then it doesn't matter, but that's probably not a good assumption to make.

@elliottsj
Copy link
Contributor

@Phoenixmatrix that's a good point. The fileInfo parameter can be used to tell whether the file is tsx:

module.exports = function transform(fileInfo, api) {
  const j = api.jscodeshift;
  if (fileInfo.path.endsWith('.tsx')) {
    j(fileInfo.source)
    // ...
  } else {
    j(fileInfo.source)
    // ...
  }
}

but module.exports.parse still somehow needs to know whether to apply the 'jsx' plugin or not.

  1. One option is to parse using recast in the transform itself:

    module.exports = function transform(fileInfo, api) {
      const parse = source => babylon.parse(source, {
        sourceType: 'module',
        plugins: file.path.endsWith('.tsx') ? ['jsx', 'typescript'] : ['typescript'],
      });
      
      const j = api.jscodeshift;
      
      return j(recast.parse(file.source, { parser: { parse } }))
      // ...
    }
  2. Another option is to modify jscodeshift and recast to pass another parameter path to the parse function:

    module.exports.parser = {
      parse: (source, path) => babylon.parse(source, {
        sourceType: 'module',
        plugins: path.endsWith('.tsx') ? ['jsx', 'typescript'] : ['typescript'],
      }),
    };
  3. Or, if we update jscodeshift to use babylon@7, we could make jscodeshift smart enough to apply the 'jsx' plugin automatically when parsing a file with extension .tsx, e.g. here.

I'm not sure which of these is better.

@elliottsj
Copy link
Contributor

I'm thinking option 3 is probably what we want for --parser babylon once jscodeshift is on babylon@7. But option 1 feels like a good workaround in the interim.

@fkling what do you think?

@RupenAnjaria
Copy link

I tried Option 3 by upgrading beta version of babylon
npm install babylon@7.0.0-beta.8 - transformation didn't work.

Also tried option 1. Updated manual-bind-to-arrow.js file, however that did not updated a single file.

command: jscodeshift -t transforms/manual-bind-to-arrow.js 'C:\\Users\\name\\Project\\proj1\\ForUpgrade\\src'

Updated file is:

export default function transformer(file, api) {

  const parse = source => babylon.parse(source, {
    sourceType: 'module',
    plugins: file.path.endsWith('.tsx') ? ['jsx', 'typescript'] : ['typescript'],
  });

  const j = api.jscodeshift;

   var root = j(file.source);
.
.
.
if (hasChanged) {
    //return transform.toSource();
    return j(recast.parse(file.source, { parser: { parse } }))
  }```

@RupenAnjaria
Copy link

RupenAnjaria commented Feb 13, 2018

Question: Our current project runs on React 15.4.2. We identified that only following transforms are required:

  1. manual-bind-to-arrow
  2. pure-component
  3. pure-render-mixin
  4. rename-unsafe-lifecycles
  5. react-to-react-dom

However, as it is Typescript - codemode does not work. It is advisable to simply upgrade without transformation? If not then manual find and replace is the only option for now.

@elliottsj
Copy link
Contributor

elliottsj commented Feb 14, 2018

@RupenAnjaria you will also need to use the work-in-progress versions of ast-types and recast, which are not yet done nor released:

You will probably also need the latest version of babylon@7, which is currently 7.0.0-beta.40.

Those transforms you mentioned probably also won't work with TypeScript sources, since they're designed around a JS AST; TypeScript type annotations will likely not be preserved.

TypeScript support is very much in a work-in-progress state right now, so I wouldn't recommend using it for any purpose beyond helping to test the pending ast-types and recast changes.

@Phoenixmatrix
Copy link

so what's interesting with the way Recast works, is that it only cares about the bits you modify. So if you modify something that's in javascript, and ignore types (eg: you rename an ES6 class. The transform does nothing to the type annotations on the class, just its name), the AST node that's changing (ClassDeclaration) is the same whether you're in typescript, flow or vanilla JS. The part that is typescript specific (TSTypeParameterDeclaration) will remain untouched.

That was the idea behind using Babylon as the parser for TypeScript in Recast instead of using TypeScript's parser itself. It should just work as long as everything in the file is defined in ast-types (and we think we have everything there) and anything you touch (only things you touch!) is in Recast. Since preexisting codemods only touch stuff recast supported all along, they should "just work"

@elliottsj
Copy link
Contributor

When playing around in https://github.com/elliottsj/jscodeshift-transforms, I did notice that creating a new identifier with j.identifier() omits the type annotation node from the original identifier:

p => j.identifier(p.node.name.split('').reverse().join(''))

e.g. let foo: string = '' would be transformed to let oof = ''

To preserve the type annotation, I had to explicitly specify typeAnnotation:
https://github.com/elliottsj/jscodeshift-transforms/blob/753bae4b18dcd622992d4b0d9ab0208ce52b50c8/transforms/reverse-identifiers.js#L24-L27

Does this mean the Identifier type in ast-types needs some changes in order to preserve the type annotation?

@Phoenixmatrix
Copy link

Phoenixmatrix commented Feb 14, 2018

if you create a new one you'll lose stuff unless you add the properties to it. You'd need to take the existing identifier and only modify the part you care about. Maybe the identifier method is lossy when cloning the nodes?

elliottsj added a commit to elliottsj/ast-types that referenced this issue Feb 26, 2018
With `.from`, you can be explicit about which fields you want to
populate, without having to deal with the positional arguments
of the normal builder.

See also:
facebook/jscodeshift#180 (comment)
ehzhang pushed a commit to ehzhang/jscodeshift that referenced this issue Feb 27, 2018
Yay!
Typescript support in the latest recast, but not in the public
jscodeshift
facebook#180
benjamn pushed a commit to benjamn/ast-types that referenced this issue Mar 9, 2018
With `.from`, you can be explicit about which fields you want to
populate, without having to deal with the positional arguments
of the normal builder.

See also:
facebook/jscodeshift#180 (comment)
elliottsj added a commit to elliottsj/jscodeshift-typescript-example that referenced this issue Apr 6, 2018
This is necessary to conditionally include the 'jsx' babylon plugin
when transforming a .tsx file. See also:
facebook/jscodeshift#180 (comment)
@elliottsj
Copy link
Contributor

I've updated my small example to use option 1 from this comment, while babylon@7 remains in beta: https://github.com/elliottsj/jscodeshift-transforms/blob/69f1d34effc22a234e41c2b2c2177f88ec8a89cd/transforms/reverse-identifiers.js#L18-L35

So this means that jscodeshift can now be used to transform TypeScript sources as long as you parse using recast directly in your transform, as shown in my example.

I also have an update pending for https://astexplorer.net/ to make it possible to parse using recast + babylon@7, which will help when authoring transforms: fkling/astexplorer#301

@blaster151
Copy link

What blockers remain on getting PR 301 merged over at astexplorer? The new capability would be extremely useful to me!

@vic08
Copy link

vic08 commented Apr 27, 2018

@elliottsj , can we already use it? can't find anything typescript related in docs

@oliviertassinari
Copy link

oliviertassinari commented May 15, 2018

I'm assuming this issue is close because the maintainers don't want to support TypeScript. Well, I believe that TypeScript market share is >30% among React users. While I don't use TypeScript, maybe supporting it is important.

@Phoenixmatrix
Copy link

@oliviertassinari it's not first class citizen, but it CAN be used with TS now. If anything, the only missing things (I think?) are documentation and possibly things to make it easier. But TS does work with it.

@oliviertassinari
Copy link

@Phoenixmatrix Oh, this is great, documenting it would be perfect. I had a different feedback from @lookfirst on Material-UI.

@ianwremmel
Copy link

Docs would be amazing. I tried @elliottsj's example a few weeks ago but it didn't seem to work.

@kiliancs
Copy link

Yeah, with @elliottsj's example I get an AssertionError [ERR_ASSERTION]: false == true.

@elliottsj
Copy link
Contributor

elliottsj commented Jun 23, 2018

I've updated my example to be simpler; please try it out and let me know if something doesn't work: https://github.com/elliottsj/jscodeshift-typescript-example

I'd be interested to hear the jscodeshift maintainers' thoughts on the level of support they'd be willing to give to TypeScript, with respect to documentation, testing, and potentially as a first-class format. i.e. without the need to define a custom parse function as is currently required:

const parse = source => babylon.parse(source, {
  sourceType: 'module',
  plugins: file.path.endsWith('.tsx') ? ['jsx', 'typescript'] : ['typescript'],
});

cc @fkling

On a related note, I learned today about this project: https://github.com/square/babel-codemod

It looks like they recently added TypeScript support (codemod-js/codemod#137)

@kiliancs
Copy link

Thanks for the update @elliottsj. With the last commit the actual transform doesn't change, right? I'm getting the same result as before: AssertionError [ERR_ASSERTION]: false == true

Some more details:

⋊> ~ jscodeshift --version
jscodeshift: 0.5.1
 - babel: 6.26.3
 - babylon: 7.0.0-beta.47
 - flow: 0.75.0
 - recast: 0.15.0
⋊> ~/P/d/t/fixer on bulk_tsfy ⨯ jscodeshift --extensions ts --dry -t ./reverse-identifiers.js ./reverse-identifiers.input.ts
Processing 1 files... 
Spawning 1 workers...
Running in dry mode, no files will be written! 
Sending 1 files to free worker...
 ERR ./test.ts Transformation error
AssertionError [ERR_ASSERTION]: false == true
    at new Patcher (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/patcher.js:20:10)
    at /home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/patcher.js:174:19
    at maybeReprint (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/printer.js:97:41)
    at print (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/printer.js:77:29)
    at exports.printComments (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/comments.js:324:22)
    at printWithComments (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/printer.js:63:16)
    at print (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/printer.js:68:20)
    at Printer.print (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/lib/printer.js:120:21)
    at Object.print (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/node_modules/recast/main.js:6:33)
    at Collection.toSource (/home/kilianc/.nvm/versions/node/v8.4.0/lib/node_modules/jscodeshift/src/Collection.js:179:21)
All done. 
Results: 
1 errors
0 unmodified
0 skipped
0 ok
Time elapsed: 0.880seconds 

The error is in the toSource call. If parse, find, replace but then just return some string, jscodeshift succeeds:

  j(recast.parse(file.source, { parser: { parse } }))
    .find(j.Identifier)
    .replaceWith(
      p => ({
        ...p.node,
        name: p.node.name.split('').reverse().join('')
      })
    );

    return "console.log('hi');";

Looking at the AST generated, it seems fine to me.

@OliverJAsh
Copy link

I just tried your example @elliottsj and I'm getting the same error as @kiliancs.

@matthewrobertson
Copy link

matthewrobertson commented Jul 18, 2018

I did this in a local fork and it was sufficient for the simple codemods I needed to run: #269

@timofei-iatsenko
Copy link

Thanks for great job provided for implementing typescript.

But it still not clear, is it possible to develop codemods using astexplorer? I saw @elliottsj PR to astexplorer project which is already merged.

But looks like the trick with using babel 7 in codemod itself are not applicable in astexplorer.

@gcoombe
Copy link

gcoombe commented Oct 4, 2018

I'm still seeing the AssertionError [ERR_ASSERTION]: false == true error when trying the @elliottsj solution. Has anyone found a workaround?

@elliottsj
Copy link
Contributor

Sorry I've neglected this thread for a while. Looks like TypeScript support was officially released today: https://github.com/facebook/jscodeshift/releases/tag/v0.6.0

Thanks @brieb @fkling!

I'm updating my example to use the new version, pending this fix.

divanutq added a commit to divanutq/ast-types that referenced this issue Aug 1, 2024
With `.from`, you can be explicit about which fields you want to
populate, without having to deal with the positional arguments
of the normal builder.

See also:
facebook/jscodeshift#180 (comment)
citiranom2w added a commit to citiranom2w/ast-types that referenced this issue Aug 4, 2024
With `.from`, you can be explicit about which fields you want to
populate, without having to deal with the positional arguments
of the normal builder.

See also:
facebook/jscodeshift#180 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests