facebook / jscodeshift

A JavaScript codemod toolkit.
https://jscodeshift.com
MIT License
9.23k stars 477 forks source link

Discussion on how to integrate with Babel's ability to use recast #168

Open hzoo opened 7 years ago

hzoo commented 7 years ago

So after https://github.com/babel/babel/pull/3561 allows babel to use recast as the parser/generator and the other prs fixed the babel 6 ast nodes for ast-types/recast we should be able to use babel as the transformer for jscodeshift.

Is there anything we want to do to the api to make it easier or should users just add it in themselves? babel-core/recast are already dependencies, but yeah it's not necessary (Ah I see babel-core is v5 for the babel 5 parser). Maybe it's just a docs change.

module.exports = function(fileInfo, api) {
  return api.jscodeshift(fileInfo.source)
    .findVariableDeclarators('foo')
    .renameTo('bar')
    .toSource();
}
const transform = require('babel-core').transform;
const recast = require('recast');
const plugin = require('./plugin');

module.exports = function(fileInfo, api, options) {
  return transform(fileInfo.source, {
    parserOpts: {
      parser: recast.parse
    },
    generatorOpts: {
      generator: recast.print
    },
    plugins: [plugin]
  }).code;
};

// ./plugin.js
module.exports = {
  visitor: {
    VariableDeclarator({ node }) {
      if (node.id.name === "foo") {
        node.id.name = "bar";
      }
    }
  }
};

Also interesting sidenote would be using the jscodeshift type api in babel

cc @fkling, @cpojer, @drewml

benjamn commented 7 years ago

My two cents:

The Visitor-based transformation API used by Babel tends to be better for complicated transforms, and transforms that need to be run by other people who don't fully understand what's happening behind the scenes, which means handling more edge cases automatically, and not giving up in cases when it's easier to edit the code by hand.

The Collection-based API is much better for hand-supervised codemods, and for transforms where it's ok if you don't handle every edge case, because you know that certain patterns of syntax are rare in your specific codebase.

These may not be the only possible options in the API landscape, but they've proven effective in their respective niches. I don't think it necessarily makes sense to unify them into one API, but I'm totally in favor of making both kinds of transforms easier to write, if possible.

fkling commented 7 years ago

Is there anything we want to do to the api to make it easier or should users just add it in themselves?

While I understand that having everything shipped with jscodeshift could be more convenient, I'm also a bit afraid of overloading it with too many features and thus making it more confusing.

I think it would be more valuable over all if we make it easier to "load" transforms from locally or globally installed packages, rather than reading a file. That would make it easier to distribute codemods with dependencies, such as in your example or what is proposed in #162 (assuming this becomes its own package).

We should also have more examples of transforms with dependencies.

Maybe we could have a separate babel-jscodeshift package that simply wires recast and babel together, so you only have to load that one? Something like:

const transform = require('babel-jscodeshift');

module.exports = function(fileInfo, api, options) {
  return transform({
    visitor: {
      VariableDeclarator({ node }) {
        if (node.id.name === "foo") {
          node.id.name = "bar";
        }
      }
    }
  });
}
eventualbuddha commented 7 years ago

I've been experimenting with writing a runner that provides the same role as jscodeshift but expects to be given babel plugins. It automatically uses recast as the parser/printer. It's frustrating, because it's so close to both jscodeshift and babel-cli. For lack of a better name, so far it's called babel-jscodeshift, but I think @fkling's use of that name is more appropriate. The help output is shown below. The only thing I haven't decided yet is how to pass options to plugins. Does this seem sane or should I be directing my energies elsewhere?

$ babel-jscodeshift -h
babel-jscodeshift [OPTIONS] [PATH … | --stdio]

OPTIONS
  -p, --plugin PLUGIN     Transform sources with PLUGIN (allows multiple).
  -r, --require PATH      Require PATH before transform (allows multiple).
      --extensions EXTS   Comma-separated extensions to process (default: ".js,.jsx").
  -s, --stdio             Read source from stdin and print to stdout.
  -h, --help              Show this help message.

EXAMPLES
  # Run with a relative plugin on all files in `src/`.
  $ babel-jscodeshift -p ./typecheck.js src/

  # Run with a plugin in `node_modules` on stdin.
  $ babel-jscodeshift -s -p babel-plugin-typecheck <<EOS
  function add(a: number, b: number): number {
    return a + b;
  }
  EOS

  # Run with a plugin which itself is transpiled using babel.
  $ babel-jscodeshift -r babel-register -p ./some-plugin.js src/

  # Run with a plugin written in TypeScript.
  $ babel-jscodeshift -r ts-node/register -p ./some-plugin.ts src/
kentcdodds commented 7 years ago

@eventualbuddha, happy to see you working on this! I agree that the name babel-jscodeshift makes more sense as @fkling uses it. I would call it babel-codemod :+1:

As far as passing options, I expect most codemods to not take options, so I wouldn't mind an API that's not entirely ergonomic. Perhaps: --plugin-option-pluginName-nameOfOption 2 which would pass nameOfOption: 2 to pluginName?

fkling commented 7 years ago

@eventualbuddha, @kentcdodds

First I want to say that I'm happy about everything that makes codemodding easier for everybody.

Does this seem sane or should I be directing my energies elsewhere?

In the light of requests such as #179 I hope we can consolidate our efforts and benefit from each other's work. I think it would make sense to have a single runner that is tool agnostic (for example the jscodeshift runner but without including jscodeshift and passing it to the transform). There are many features the runner could provide that are useful regardless of the actual transformation tool. E.g. I like your -p option to load a node module, that's something that's been missing from jscodeshift so far. Other features are reporting, dry run capabilities, etc.

A picture is always better:

┌─────────────────────────┐                     ┌─────────────────────────┐
│                         │                     │Adapter                  │
│                         │                     │(e.g. Babel, jscodeshift)│
│                         │   ───────────────▶  │                         │
│         Runner          │       Protocol      │ ┌─────────────────────┐ │
│                         │   ◀───────────────  │ │                     │ │
│                         │                     │ │      Transform      │ │
│                         │                     │ │                     │ │
│                         │                     │ └─────────────────────┘ │
└─────────────────────────┘                     └─────────────────────────┘

The runner communicates with the transform (or the adapter, more on that below) via a protocol. In jscodeshift, the protocol is very simple atm: A string is passed to and returned from the transform. But in the future it might be more complex, e.g. the transform returns the AST instead so that the user has more control over how it is printed, or that the AST can be reused across multiple transforms.

The transform takes whatever the runner passes to it (e.g. the source of the file or later an existing AST) and does something with it.

The (optional) adapter is an intermediate layer between the runner and the transform. It can pass additional data to the transform, such as the tool to use (babel, jscodehift) or implement the protocol completely if the transform doesn't speak it (e.g. for existing babel plugins).

The adapter could be used programmatically in a transform, similar how I showed it above with the babel-jscodeshift example, but it could also be specified on the command line, e.g.

codemod -a babel -p babel-plugin-typecheck ./foo.js
# or
codemod -a jscodeshift -t ./transform.js ./foo.js

In the future, the runner could then even be used to run transformations written in other languages (for other languages) (if the adapter takes care of evaluating the transform).


What do you think?

cpojer commented 7 years ago

@fkling actually, the natural choice should be Jest's runner; it doesn't require a lot of work to make it more generic.

eventualbuddha commented 7 years ago

I've open-sourced what I have at https://github.com/square/babel-codemod. PRs/suggestions/comments welcome!

fkling commented 7 years ago

@eventualbuddha, what are your thoughts on my previous comment?

eventualbuddha commented 7 years ago

what are your thoughts on my previous comment?

I like the idea. As long as we keep perfect from being the enemy of the good ;)