Rewriting Lebab as a Babel transformer?

[mohebifar]

Currently lebab is doing so much from parsing to generating code. During the process, somethings are missing (Like nice warnings while parsing the code #136 or JSX Support #130) or even some bugs show up. I believe almost all of these issues have been considered and solved in babel.

Lebab's aim is basically transforming an old code to a modern code. That means the only thing that matters in lebab is transforming. So, making a babel transformer instead of re-implementing all that stuff can be an option for the future releases.

[nene]

That's an intriguing idea, but I to my understanding Babel does not fully preserve the original whitespace. For example, when transforming the following code, that has nothing to transform:

/**
 * My func
 */
function  foo   (y) {
    // Hello world
    if (  true  ) {
            // poor indentation
    }
}

Babel produces:

/**
 * My func
 */
function foo(y) {
  // Hello world
  if (true) {
    // poor indentation
  }
}

It does preserve all the comments, but it pretty much reformats all the whitespace. This behavior is fine for Babel, as one only cares that the output is readable.

In Lebab's case you'll want the output to be 100% the same as input when no transforms were done - so Lebab would not mess up whatever strange coding convention one has, and you could look at the diff of code before/after and only see the actual transformations without any whitespace noise.

So it seems to me that this is not a feasible approach until Babel adopts a code generator more similar to Recast.

[nene]

On further investigation I discovered that it might be feasible to use the parser of Babel - Babylon. The AST it generates is mostly compatible with ESTree format used by Esprima and others, but there are some minor differences.

[benjamn]

Recast is parser-agnostic; you can call recast.parse(source, { parser: whatever }), as long as whatever is an object with a .parse method that produces AST nodes with .loc information.

[nene]

@benjamn Yep, Lebab is already using that run Espree parser instead. The problem with Babylon is that it doesn't produce AST that's fully compatible with ESTree spec.

[hzoo]

@benjamn So we could switch out babel-generator for recast.print to keep original formatting (assuming you wanted something like jscodeshift/eslint autofix/run babel like babel src --out-dir src?

[hzoo]

A babel transform/babel = babylon + babel-traverse + babel-generator so you could modify the ast with a babel transform and output to something that can consume a babel AST (recast)?

[benjamn]

Well, Babel is optimized for code generation performance, and preserving formatting takes a bit more effort, but recast.print will work with any AST that recast.parse parsed (as long as the nodes have .loc.{start,end} information).

[hzoo]

Ok I'm just wondering what we can do to make recast + babel interop better if that's possible. I think it would be nice to be able to write a babel transform like babel src -d src --presets=lebab --generator=recast and it uses recast instead of babel-generator to preserve source (codemod, autofix, etc). I've experimented with simple transforms like http://astexplorer.net/#/sskXYR4icD/2

[benjamn]

If this works (and I don't know why it wouldn't), then it shouldn't be hard:

var recast = require("recast");
var ast = recast.parse(source, { parser: require("babylon") });
var result = require("babel-core").transformFromAst(ast, {
  presets: ["lebab"],
  ast: true
});
console.log(recast.print(result.ast).code);

[hzoo]

@benjamn would recast have to parse it like recast.parse(source, { parser: require("babylon") }); or can you still use babylon on its own? Just wanted to know what changes would need to be made to babel to support this for a regular babel setup (sounds like we would need an option to use a different parser/generator in babelrc)?

[benjamn]

Letting recast.parse do the parsing is not essential, in principle, but it does have several advantages:

• The recast.parse code can pass the options it needs to the parser, reducing manual configuration.
• The .loc.{start,end} information in the parsed AST is guaranteed to correspond to the original source, since no other code has had the chance to modify the AST. In some cases recast.parse even fixes bugs related to .loc information for the various Esprima-like parsers that it allows.
• The real magic of recast.parse is that it makes a copy of the AST, and links every node in the copy to its original node via an .original property. This shadow-AST is what enables recast.print to perform tree diffing and reuse original source code when possible.

In other words, you could parse the AST yourself, and then call some other recast.* API to make the copy and create the .original linkage, but recast.parse is a simpler interface for that, I think.

[hzoo]

Ok I can test it my modifying babel's transformation step to take in a different parser/generator.

I think the last thing is to accept babel AST nodes in ast-types? Since Error: unknown: {type: ObjectProperty} does not match type Printable Or just modify them like before? (Looked at babel/babel@eac9e9b) seems to work as a experiment

I recall benjamn/ast-types#132 too - should we make separate types for babel's 6 ast differences?

[hzoo]

@benjamn yay something like this seems to work babel/babel#3561

or it would be cool if we could subsitute with babel-types

@mohebifar @nene I can help with converting when it's possible. Lebab can be a babel preset with plugins (could be a monorepo with lerna as well)

[hzoo]

As a update for this to happen:

• babel-core: add options for different parser/generator babel/babel#3561 in babel-core needs to land in some form (probably a option to pass a string (similar to a plugin) that resolves to a module with a parse method and a generator method) - I'm testing in my local astexplorer
• Babel 6 types benjamn/ast-types#162 update ast-types with babel 6 types - (need to fix some tests)
• Babel 6 changes + flow benjamn/recast#299 update recast printer with the same types - (need to fix some tests)

[hzoo]

babel/babel#3561 has landed! So writing a plugin that doesn't include the different AST nodes should work now https://github.com/babel/babylon#output

[hzoo]

Merged the 2 prs ^ so this is actually a possibility now! example here: fkling/astexplorer#161

[hzoo]

Let me know if you want to discuss doing this at some point again! I know there's a lot on everyone's plate (and it will take a good amount of work) but just checking the option open (now that this can be done). Can also chat on our slack: http://slack.babeljs.io/

Could be a good opportunity for some simple beginner-friendly contribution issues as well if everything is planned out. cc @nene @mohebifar

Could even try using https://github.com/jlongster/prettier as the generator but will require some testing

[nene]

I actually started out with switching the parser in Lebab over to Babylon. Wrote an adapter to convert Babylon node types to Esprima node types, this got most of the tests passing, but that's really a temporary solution. So got a bit stuck behind the task of converting the code really over to use Babylon AST (more like running out of free time).

Thanks for the resources!

[hzoo]

Yeah definetely reach out if you have any questions/concerns: I know there's a lot to know and yes its certainly overwhelming. Slack or twitter: https://twitter.com/left_pad, https://twitter.com/babeljs.

Wrote an adapter to convert Babylon node types to Esprima node types, this got most of the tests passing, but that's really a temporary solution

I actually had to do this exact thing for https://github.com/babel/babel-eslint if you didn't know heh https://github.com/babel/babel-eslint/tree/master/babylon-to-espree.

The ast differences are https://github.com/babel/babylon#output

And also FYI we are also starting out an effort to make a plugin to output back to ESTree/Babel 5 output in the parser. babel/babylon#277 (@danez) which is basically the same logic but built-into the parser. The way we did it in babel-eslint is slow and really bad since we parse with babylon first and then change nodes around to fit the other format.

Other resources if curious: https://github.com/thejameskyle/babel-handbook/blob/master/translations/en/plugin-handbook.md, http://henryzoo.com/babel-plugin-slides, http://astexplorer.net/

---

I made a few basic POC transforms a long time ago: http://astexplorer.net/#/sskXYR4icD/1, http://astexplorer.net/#/vd18PVxJ8J/2, http://astexplorer.net/#/zdTZE92PrZ/2, http://astexplorer.net/#/3sxHbt9I7u/1 (although not sure I'd do it the same way now)

We can always add some better api methods

[nene]

I wonder how well does something like this babylon-to-espree work? Like what are the downsides of not using Babylon AST directly? For Lebab the speed isn't really a crucial aspect. Perhaps I'm better off with an adapter layer like this.

I guess my main concern is coupling Lebab too tightly with the non-standard AST tree of Babylon, which is likely to fluctuate more than ESTree spec. Though I have to say I like some of the AST nodes of Babylon much more than their ESTree equivalents (especially the separate Literal nodes).

[nene]

Rewriting Lebab transforms as Babel transforms is another direction I'd really like to explore. But that will be a larger undertaking. I've accumulated a fair chunk of code in all these transforms. But I've also come across some of the limitations of the current system - especially with a need to track variables. Some sort of evolutionary approach would be nice to gradually reimplement the transforms as Babel-transforms, while keeping the others working alongside.

[hzoo]

Yeah the only downsides are speed plus hard to maintain + not enough tests (just a time/resource issue).

I guess my main concern is coupling Lebab too tightly with the non-standard AST tree of Babylon, which is likely to fluctuate more than ESTree spec.

We aren't planning on making anymore changes

Though I have to say I like some of the AST nodes of Babylon much more than their ESTree equivalents (especially the separate Literal nodes).

Yeah the changes I think are definetely nice especially for transforming which makes sense for Babel and for the AST, but in the end it doesn't interop well unless everyone agrees on doing ESTree 2.0

[hzoo]

Is this still something we want to do for Lebab? I was wondering if we could ask this ask as part of https://developers.google.com/open-source/gsoc/?

[nene]

I do still have very much interest in doing this. Though I have drawn myself back from various open-source developments lately to just spend more time in the real world.

Anyway, I have nothing against proposing this task for Google Summer of Code. It might even be better this way as I've found I'm better at cleaning up the code others have written than writing new stuff by myself from scratch.

[hzoo]

I do still have very much interest in doing this.

Cool 😄

Though I have drawn myself back from various open-source developments lately to just spend more time in the real world.

Totally understandable and a good thing!

Awesome, yeah I'll add it to the list, and hopefully we get some volunteers on this effort then. https://github.com/babel/babel/wiki/Google-Summer-of-Code-Proposals

We'll just want to plan out how it should happen (I think we would all want it to be incremental so it's easy to review)

[rajatvijay]

Hi guys, I would like to work on this project under GSOC. Lately I have been learning AST and how to write plugins for babel.
Any help like even pointing out to resources to get started will be of great cause. Thanks in advance!

[nene]

Hi there. I was busy slacking off during weekend :)

Now finally found some time to review your code. Wrote quite a lot of comments. Unfortunately Github gists don't allow writing inline comments for particular lines of code. Tried to make my best without that.

See: https://gist.github.com/nene/d9d9ec9802fd6e38cb4e212b8ca90b84

[nene]

Also, one thing that we should consider rather sooner than later is the fact that ES6 imports are effectively const, so the following code should not be transformed:

var foo = require("foo");
...
foo = somethingElse();

Dealing with variable scoping is currently one of the main weaknesses of Lebab. Babel should provide us with much better scope information.

[rajatvijay]

Hi @nene
Very very thanks for the review. It changed my way of looking at the problem in hand.

Sorry for replying so late. Got busy in some college exams.

Improving upon the points mentioned. And really like the "patterns matching in trees" blog post. It can help reduce a lot of code.

Will start off by extracting that feature as a separate library from lebab so can be used in the current problem and such problems coming forth.

[rajatvijay]

@nene

I have one doubt.

Dealing with variable scoping is currently one of the main weaknesses of Lebab. Babel should provide us with much better scope information.

Why need a better scope information if babel API handles it well itself?

[nene]

What I meant was that scope information provided by Babel seems to be better than the one we currently in Lebab get through a separate helper library.

[rajatvijay]

Hi @nene

Apologies for disappearing had some personal crises.

Wanted to confirm if this task is still up for grabs or can I resume working on it, irrespective of any summer of code challenge.

Thanks

[nene]

Hi @rajatvijay, you are very much free to continue working on this.

Barely anything has changed in the mean time.

[hzoo]

Yep, can totally be done outside of any program, was just a suggestion. Maybe can look at https://github.com/square/babel-codemod as well by @eventualbuddha

[hzoo]

This would still be amazing to do, for RGSoC we have a codemod for a TC39 proposal WIP which also could be a good candidate for this babel/babel#6048

[abiduzz420]

Hello @nene @mohebifar I find lebab to be a really handy tool for doing large code base migrations from 5to6. I would like to talk to you on the subject of using lebab as a babel preset.
I have few questions which could help us understand about the idea better:

I would like to know if this idea be implemented ? Can we make the use of tool better, if we internally integrate with babel without the user having to configure anything extra. Since this would involve re-writing of majority of transformations, I wish to hear your thoughts before moving further.
Thanks in advance :)

EDIT: One of the reasons I am very interested about this project is the fact that it deals with writing code-mods and lets developers perform pain less migrations. Second reason is writing the project in the form of plugin based architecture. This would help us isolate one transformation and work on it to support latest changes made by tc39. Thirdly, it would remain in parity with the rest of the babel ecosystem. A developer would not need to configure anything provided he/she has babel already setup.

[nene]

Hi @abiduzz420 I don't quite understand what would be the benefit of implementing Lebab as a Babel preset. As I see it, to use a babel preset you would actually need to configure more than you currently need to do with Lebab (really, no configuration at all). And they way you use Babel is pretty different from the way you would use Lebab. You would use Lebab to perform the migration once, after which you won't have much use for it any more. In contrast to Babel that you'd keep continuously running. Also with Lebab you usually want the transforms to modify the existing files, versus Babel, where you'd rather output the generated sources to another file.

Perhaps I misunderstood you. I have nothing against switching to Babylon parser and making use of some other existing Babel-related tools.

[abiduzz420]

I agree with you. Lebab is used as a one time tool and that it would require more configuration to set it up as preset rather than using it the way it is (no config).

[hzoo]

I tried to explain this in babel/babel#7296 (comment) and here already but I only thought we wanted to change Lebab internally to use babel, the cli would be the same, no configuration needed etc. Technically this could simply just mean creating each transform as a plugin (with a preset) and to turn on or turn off each plugin (or plugin option) by mapping those to the cli options.

And the purpose of this is to support newer syntax in the parser as well as take advantage of tooling used in babel. The user doesn't need to know this change happens so it's basically a refactor. Lebab could just be a wrapper around a preset and run it (think of it as another babel-cli)

[abiduzz420]

Yes, this suddenly makes a lot of sense to me now (using it as another babel-cli). I did not make myself clear when I said plugin based transforms and keeping the cli intact.
I understand the problem statement better now.

[eventualbuddha]

I would be very interested in seeing whether babel-codemod would work as a base for “lebab as a preset”. It should be fairly easy to do, and I can put together an example runner with an existing babel preset if that would help.

This approach would be very similar to what @hzoo described as lebab being “another babel-cli”, as that’s pretty much what babel-codemod is, but with recast as a parser/printer to preserve as much formatting as possible.

[rajatvijay]

Hey @nene @hzoo

I have put together a babel plugin to transform require calls to import calls. Here is the repo for it.
https://github.com/rajatvijay/babel-plugin-transform-require.

The plan is to compose lebab from such babel plugins and wrap it inside the lebab cli utility for backward compatibility.

Let me know in what ways this plugin can be improved and should I move ahead in this direction to make others plugins too.

[nene]

@rajatvijay This looks like a great start!

A few things that immediately pop to my mind:

• We should copy the existing Lebab tests over and make them work with this plugin. Replacing the manually runnable tests in try/ dir with automated ones.
• The matchesAst and friends should really be moved to a package of its own. I've been planning to do that, but haven't so far had a good motivation for doing so. I guess now it's finally time for that.
• The repo also contains the compiled sources in lib/. That's usually not a good idea.

[nene]

@rajatvijay I extracted the matchesAst() helper out of Lebab and published in npm under name f-matches.

While doing so I changed the API slightly:

• The module exports three functions: matches, matchesLength, extract.
• So mainly I've dropped the AST-part from function name, as there really isn't nothing specific to AST nodes there.
• isAstMatch() has been eliminated by making matches() a curried function. Which means that the argument order is swapped now.
• The two other functions are also curried.

In short, you should be able to just replace matchesAst with matches imported from f-matches package.

[rajatvijay]

@nene

The repo also contains the compiled sources in lib/. That's usually not a good idea.

This has been fixed.

I extracted the matchesAst() helper out of Lebab and published in npm under name f-matches.

This is going to be my next step.

1. Integrate this library into the plugin
2. and then will migrate the tests too

[rajatvijay]

@nene the plugin has been published to npm with all the tests passing and using f-matches.

Moving on to the new transform.

[eventualbuddha]

Might I suggest following babel’s lead and using a monorepo for the lebab-as-babel-plugins packages?

[rajatvijay]

@eventualbuddha seems like a valid thing to do. Let me do a little research on monorepo, its cons, and management, then I can create the repo and place the babel-plugin-transform-require in that repo.

[niieani]

I recommend using yarn workspaces instead of things like lerna. It does almost everything necessary and works much better overall in my experience.

[eventualbuddha]

This is an explanation of how we use a monorepo with yarn and Ember. Not that much is Ember-specific.

https://medium.com/square-corner-blog/ember-and-yarn-workspaces-fca69dc5d44a

[rajatvijay]

Thanks @niieani and @eventualbuddha. I have successfully made a monorepo for this, here

@nene: I was working on transform-export plugin and was looking at the tests and got confused:

it('should ignore function export when function name does not match with exported name', () => {
      expectNoChange('exports.foo = function bar() {};');
});

Shouldn't this be transformed to export {bar as foo}?
Here is the link to these tests.

[eventualbuddha]

Shouldn't this be transformed to export {bar as foo}?

Maybe. It depends on what else is in the file. If that were the only thing, then yes, and that's what esnext does.

As long as you aren't changing the behavior of the program unexpectedly, you should make what upgrades you can. In this case, as long as there is no existing bar in scope or any references to a global bar, you can make function bar() {} be a FunctionDeclaration instead of a FunctionExpression and then use the export declaration you indicated.

I recommend checking out the esnext tests and possibly implementation to help you.