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

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

Open
hzoo opened this Issue Nov 8, 2016 · 9 comments

Comments

Projects
None yet
6 participants
@hzoo
Contributor

hzoo commented Nov 8, 2016

So after babel/babel#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

This comment has been minimized.

Show comment
Hide comment
@benjamn

benjamn Nov 8, 2016

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.

benjamn commented Nov 8, 2016

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

This comment has been minimized.

Show comment
Hide comment
@fkling

fkling Nov 9, 2016

Member

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";
        }
      }
    }
  });
}
Member

fkling commented Nov 9, 2016

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

This comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Mar 6, 2017

Contributor

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/
Contributor

eventualbuddha commented Mar 6, 2017

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

This comment has been minimized.

Show comment
Hide comment
@kentcdodds

kentcdodds Mar 16, 2017

@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 👍

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?

kentcdodds commented Mar 16, 2017

@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 👍

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

This comment has been minimized.

Show comment
Hide comment
@fkling

fkling Mar 16, 2017

Member

@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?

Member

fkling commented Mar 16, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Mar 17, 2017

Contributor

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

Contributor

cpojer commented Mar 17, 2017

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

@eventualbuddha

This comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Mar 28, 2017

Contributor

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

Contributor

eventualbuddha commented Mar 28, 2017

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

@fkling

This comment has been minimized.

Show comment
Hide comment
@fkling

fkling Mar 28, 2017

Member

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

Member

fkling commented Mar 28, 2017

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

@eventualbuddha

This comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Mar 28, 2017

Contributor

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 ;)

Contributor

eventualbuddha commented Mar 28, 2017

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 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment