Add namespace removing transform #140

BrendanAnnable · 2016-06-13T15:46:19Z

Motivation: Updating old JavaScript code to ES6 can often have namespaces (related issue). e.g.

FirstNS.SecondNS.MyClass = function(a, b) {
};

FirstNS.SecondNS.MyClass.prototype.myFunc = function(a, b) {
};

where you would prefer:

class MyClass{
  constructor(a, b) {
  }

  myFunc(a, b) {
  }
};

My first approach was to introduce this into the class transform, but I believe the approach in this PR separates the concerns a bit better by introducing a transform whose sole job is to remove namespaces.

Unfortunately this introduces an ordering dependency, where you need to run no-namespace transform before running the class transform. I'd like to hear feedback on this.

The code submitted is really my first hacky attempt, without thorough testing. Hoping this PR generates some interest and maybe some ideas on how to add this pretty handy feature.

Edit: Oh and the Error: {...} does not match type string error was super annoying, I eventually commented out isString.assert(stmt); in recast/lib/printer.js because I couldn't tell what was causing it. Maybe you will know :)

Edit: More TODOs: Update usage throughout file, and preserve comments.

BrendanAnnable · 2016-06-13T15:48:07Z

src/transform/no-namespace.js

+      type: 'MemberExpression',
+      computed: false,
+      property: ast => {
+        if (ast.type === 'Identifier' && ast.name !== 'prototype' && ast.name[0] === ast.name[0].toUpperCase()) {


So the casing check sucks, but basically I can't tell the difference between these two:

FirstNS.MyClass.myStaticFunc = function(a, b) { }; FirstNS.SecondNS.MyClass = function(a, b) { };

nene · 2016-06-13T16:37:22Z

Thanks for all these contributions!

I have some doubts about the usefulness of this transform. The main problem is that this transform deliberately breaks code. So far none of the transforms in Babel do this.

For this to really be useful, it would have to also transform the code that uses these namespaced classes. But even then it wouldn't fully work... it would just result in name conflicts as the namespaces have been removed. I guess as the global namespace is used to share the classes, we'd have to replace the namespaces with modules - converting the class declarations to exports and adding imports to files that use the classes... that in turn would need the knowledge of the files/directories layout. This is getting kinda big and hairy...

I'll take closer look at this, but these are my initial thoughts.

BrendanAnnable · 2016-06-14T00:23:15Z

So I agree, but to your point about converting namespaces to imported/exported modules, that is exactly what I'm trying to achieve in the long run. In my case I have a large codebase using namespaced function/prototype classes, and I'm looking to move everything to ES6 with imports and exports. It definitely gets big and hairy, but I see that as the big advantage of this tool, to make that hairy process easier!

What do you think about adding the ability to easily add custom transforms that don't work in the generic case but allow me to at least create custom transforms for my own project?

I also think that transforms are going to need to be applicable to a wider context then a single file to allow these sorts of multiple-file refactors. Do you think that is a direction you see this project heading?

nene · 2016-06-14T06:25:46Z

I've been thinking of custom transforms for a while. There just hasn't been any urgency of adding support for them, as nobody has so far requested these. This pull request would be a perfect candidate for one.

The thing with multiple files is that currently Lebab works on just a single file. There's issue #137 to allow it to recurse into directories, but giving the filename info to transforms is a step even further from that. Perhaps this namespaced-classes to imported/exported-ES6-classes transformation is something that could be built on top of Lebab, that is, a separate tool that could use Lebab internally.

BrendanAnnable · 2016-07-12T01:27:32Z

So as an update, I ended up building an entirely new project to accomplish the namespace removal using the same technologies that lebab currently uses, i.e. estraverse and recast, so I'll close this PR.

I've since found the Babel Plugin Handbook which looks to already have some powerful features like detecting variable scope etc. This would likely go nicely with this suggestion #138. I'll probably start writing my own transforms as babel transforms if recast starts playing nicely with it.

Add namespace removing transform

6b9ac49

BrendanAnnable reviewed Jun 13, 2016
View reviewed changes

BrendanAnnable closed this Jul 12, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add namespace removing transform #140

Add namespace removing transform #140

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add namespace removing transform #140

Add namespace removing transform #140

Uh oh!

Conversation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!