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

Rewriting Lebab as a Babel transformer? #138

Open
mohebifar opened this Issue Jun 11, 2016 · 65 comments

Comments

Projects
None yet
8 participants
@mohebifar
Member

mohebifar commented Jun 11, 2016

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jun 11, 2016

Collaborator

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.

Collaborator

nene commented Jun 11, 2016

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jun 12, 2016

Collaborator

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.

Collaborator

nene commented Jun 12, 2016

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

This comment has been minimized.

Show comment
Hide comment
@benjamn

benjamn Jun 13, 2016

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.

benjamn commented Jun 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jun 13, 2016

Collaborator

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

Collaborator

nene commented Jun 13, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 18, 2016

@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 commented Jun 18, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 29, 2016

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

hzoo commented Jun 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@benjamn

benjamn Jun 29, 2016

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

benjamn commented Jun 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 29, 2016

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

hzoo commented Jun 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@benjamn

benjamn Jun 29, 2016

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

benjamn commented Jun 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 30, 2016

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

hzoo commented Jun 30, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@benjamn

benjamn Jun 30, 2016

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.

benjamn commented Jun 30, 2016

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 30, 2016

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 commented Jun 30, 2016

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 30, 2016

@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 commented Jun 30, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jul 15, 2016

As a update for this to happen:

  • 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
  • benjamn/ast-types#162 update ast-types with babel 6 types - (need to fix some tests)
  • benjamn/recast#299 update recast printer with the same types - (need to fix some tests)

hzoo commented Jul 15, 2016

As a update for this to happen:

  • 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
  • benjamn/ast-types#162 update ast-types with babel 6 types - (need to fix some tests)
  • benjamn/recast#299 update recast printer with the same types - (need to fix some tests)
@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 29, 2016

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 commented Sep 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Nov 4, 2016

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

hzoo commented Nov 4, 2016

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

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jan 21, 2017

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

hzoo commented Jan 21, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jan 21, 2017

Collaborator

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!

Collaborator

nene commented Jan 21, 2017

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jan 21, 2017

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

hzoo commented Jan 21, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jan 21, 2017

Collaborator

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

Collaborator

nene commented Jan 21, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jan 21, 2017

Collaborator

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.

Collaborator

nene commented Jan 21, 2017

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jan 22, 2017

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 commented Jan 22, 2017

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 17, 2017

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

hzoo commented Feb 17, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Feb 20, 2017

Collaborator

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.

Collaborator

nene commented Feb 20, 2017

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 20, 2017

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)

hzoo commented Feb 20, 2017

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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 5, 2017

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!

rajatvijay commented Mar 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 5, 2017

Collaborator

Hard to point out any general resources. If you have a specific question in mind, feel free to ask.

Collaborator

nene commented Mar 5, 2017

Hard to point out any general resources. If you have a specific question in mind, feel free to ask.

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 6, 2017

@nene thanks!
So I have tried Babel and lebab both, on the product level and went through the source code of lebab.
I think the starting point should be deciding the specs of all the babel plugins that will be developed.

If you have any other starting point in mind please let me know.

rajatvijay commented Mar 6, 2017

@nene thanks!
So I have tried Babel and lebab both, on the product level and went through the source code of lebab.
I think the starting point should be deciding the specs of all the babel plugins that will be developed.

If you have any other starting point in mind please let me know.

@nene

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 6, 2017

Collaborator

The approach I originally tried regarding this whole thing was to replace just the parser used by Lebab with Babel. It didn't really turn out quite as simple though, but I guess it would still be a feasible approach. Another side of the coin is that AST manipulation inside current Lebab code has several shortcomings. The main shortcomings are in dealing with variable scoping. This is where I think switching over to Babel API-s should improve Lebab the most.

Regarding a spec, I think the functionality of Lebab is pretty well specified by the tests. So when re-implementing Lebab functionality as a Babel plugin, one mainly needs to ensure that all the existing tests pass.

Collaborator

nene commented Mar 6, 2017

The approach I originally tried regarding this whole thing was to replace just the parser used by Lebab with Babel. It didn't really turn out quite as simple though, but I guess it would still be a feasible approach. Another side of the coin is that AST manipulation inside current Lebab code has several shortcomings. The main shortcomings are in dealing with variable scoping. This is where I think switching over to Babel API-s should improve Lebab the most.

Regarding a spec, I think the functionality of Lebab is pretty well specified by the tests. So when re-implementing Lebab functionality as a Babel plugin, one mainly needs to ensure that all the existing tests pass.

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 6, 2017

So the main project is to improve the lebab or rewrite it as a babel plugin?

rajatvijay commented Mar 6, 2017

So the main project is to improve the lebab or rewrite it as a babel plugin?

@nene

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 7, 2017

Collaborator

I think the transforms that Lebab does could be rewritten as a Babel plugins. But it would still be nice to package them inside the Lebab program, so that nothing would change for somebody already using Lebab. This way we could also replace the functionality gradually. There's quite a bit of code in Lebab. Better to tackle it one transform at a time. Of course, the Babel plugins could additionally be used separately from Lebab, if one wishes so, but I think for most people it would be simpler to use as a single program.

Collaborator

nene commented Mar 7, 2017

I think the transforms that Lebab does could be rewritten as a Babel plugins. But it would still be nice to package them inside the Lebab program, so that nothing would change for somebody already using Lebab. This way we could also replace the functionality gradually. There's quite a bit of code in Lebab. Better to tackle it one transform at a time. Of course, the Babel plugins could additionally be used separately from Lebab, if one wishes so, but I think for most people it would be simpler to use as a single program.

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 7, 2017

Okay so I think I should go forward with rewriting the lebab transforms as a babel plugin one by one, starting with the let transform. Also, will make sure that the plugins can be packaged in a lebab program so the legacy functionality doesn't break.

Please feel free to point out if there is anything that I missed. Thanks in advance!

rajatvijay commented Mar 7, 2017

Okay so I think I should go forward with rewriting the lebab transforms as a babel plugin one by one, starting with the let transform. Also, will make sure that the plugins can be packaged in a lebab program so the legacy functionality doesn't break.

Please feel free to point out if there is anything that I missed. Thanks in advance!

@nene

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 7, 2017

Collaborator

Sounds great.

Collaborator

nene commented Mar 7, 2017

Sounds great.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Mar 7, 2017

@rajatvijay If you want to do this as part of gsoc, you need to check out https://summerofcode.withgoogle.com/organizations/5842528113786880/ and the gsoc website to apply with your proposal and other information.

hzoo commented Mar 7, 2017

@rajatvijay If you want to do this as part of gsoc, you need to check out https://summerofcode.withgoogle.com/organizations/5842528113786880/ and the gsoc website to apply with your proposal and other information.

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 7, 2017

@hzoo but the applications start on March 20 right?

rajatvijay commented Mar 7, 2017

@hzoo but the applications start on March 20 right?

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 16, 2017

@nene, @hzoo Hi!
I started with writing the babel plugin to transform require calls to import declarations. Have made some progress in accordance with the tests written in lebab, would say 50% through it.
But I guess getting a code review of what I have written so far will have me straighten up my line of thought and also get a sense of a quality of my style coding, as it suits the lebab/babel community or not.
Also, I don't think generating a pull request will be a good idea since the plugin is not complete.

rajatvijay commented Mar 16, 2017

@nene, @hzoo Hi!
I started with writing the babel plugin to transform require calls to import declarations. Have made some progress in accordance with the tests written in lebab, would say 50% through it.
But I guess getting a code review of what I have written so far will have me straighten up my line of thought and also get a sense of a quality of my style coding, as it suits the lebab/babel community or not.
Also, I don't think generating a pull request will be a good idea since the plugin is not complete.

@nene

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 17, 2017

Collaborator

I'm sure I could review your code and provide some feedback.

A pull request is fine, even when it's completely broken, there's no requirement to actually merge it. Or present the code in some other form that you find more suitable.

Collaborator

nene commented Mar 17, 2017

I'm sure I could review your code and provide some feedback.

A pull request is fine, even when it's completely broken, there's no requirement to actually merge it. Or present the code in some other form that you find more suitable.

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 17, 2017

@nene Here is the link to the file that contains the plugin code.

https://github.com/rajatvijay/babel-plugin-transform-require/blob/master/src/index.js

I completely understand that it can be refactored and improved, but since it's not a complete implementation, I have majorly focussed in passing the tests.

Also made it a gist so that you can provide your comments there and then. The gist url is:
https://gist.github.com/rajatvijay/852003cb37afea370ad459c6f3de84c1#file-index-js-L1

rajatvijay commented Mar 17, 2017

@nene Here is the link to the file that contains the plugin code.

https://github.com/rajatvijay/babel-plugin-transform-require/blob/master/src/index.js

I completely understand that it can be refactored and improved, but since it's not a complete implementation, I have majorly focussed in passing the tests.

Also made it a gist so that you can provide your comments there and then. The gist url is:
https://gist.github.com/rajatvijay/852003cb37afea370ad459c6f3de84c1#file-index-js-L1

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 19, 2017

@nene Hi
Did you get a chance to see the code?

rajatvijay commented Mar 19, 2017

@nene Hi
Did you get a chance to see the code?

@nene

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 20, 2017

Collaborator

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

Collaborator

nene commented Mar 20, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 20, 2017

Collaborator

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.

Collaborator

nene commented Mar 20, 2017

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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 23, 2017

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 commented Mar 23, 2017

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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 29, 2017

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

rajatvijay commented Mar 29, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 29, 2017

Collaborator

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.

Collaborator

nene commented Mar 29, 2017

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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Jun 26, 2017

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

rajatvijay commented Jun 26, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Jun 26, 2017

Collaborator

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

Barely anything has changed in the mean time.

Collaborator

nene commented Jun 26, 2017

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

Barely anything has changed in the mean time.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 26, 2017

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 commented Jun 26, 2017

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 comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Aug 3, 2017

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

hzoo commented Aug 3, 2017

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

This comment has been minimized.

Show comment
Hide comment
@abiduzz420

abiduzz420 Feb 9, 2018

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.

abiduzz420 commented Feb 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Feb 9, 2018

Collaborator

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.

Collaborator

nene commented Feb 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@abiduzz420

abiduzz420 Feb 9, 2018

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

abiduzz420 commented Feb 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 9, 2018

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)

hzoo commented Feb 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@abiduzz420

abiduzz420 Feb 9, 2018

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.

abiduzz420 commented Feb 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Feb 9, 2018

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.

eventualbuddha commented Feb 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Mar 28, 2018

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.

rajatvijay commented Mar 28, 2018

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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Mar 28, 2018

Collaborator

@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.
Collaborator

nene commented Mar 28, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@nene

nene Apr 1, 2018

Collaborator

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

Collaborator

nene commented Apr 1, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Apr 2, 2018

@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 commented Apr 2, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Apr 4, 2018

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

Moving on to the new transform.

rajatvijay commented Apr 4, 2018

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

Moving on to the new transform.

@eventualbuddha

This comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Apr 4, 2018

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

eventualbuddha commented Apr 4, 2018

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

@rajatvijay

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Apr 4, 2018

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

rajatvijay commented Apr 4, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Apr 4, 2018

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

niieani commented Apr 4, 2018

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 comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Apr 4, 2018

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

eventualbuddha commented Apr 4, 2018

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

This comment has been minimized.

Show comment
Hide comment
@rajatvijay

rajatvijay Apr 23, 2018

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.

rajatvijay commented Apr 23, 2018

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

This comment has been minimized.

Show comment
Hide comment
@eventualbuddha

eventualbuddha Apr 23, 2018

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.

eventualbuddha commented Apr 23, 2018

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.

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