Skip to content

Instantly share code, notes, and snippets.

@nene
Forked from rajatvijay/index.js
Created March 20, 2017 07:58
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save nene/d9d9ec9802fd6e38cb4e212b8ca90b84 to your computer and use it in GitHub Desktop.
Save nene/d9d9ec9802fd6e38cb4e212b8ca90b84 to your computer and use it in GitHub Desktop.
babel-plugin-tranform-require
// Need to implement warnings
module.exports = function(babel) {
var t = babel.types;
return {
visitor: {
VariableDeclaration: function(path, state) {
var newPaths = path.node.declarations.map(function(declaration, index, declarationsArr) {
if (t.isIdentifier(declaration.id) && isTransformableFunctionCall(path, declaration)) {
return importDeclaration(t, [declaration.id], [], declaration.init.arguments[0]);
} else if (t.isObjectPattern(declaration.id) && isTransformableFunctionCall(path, declaration)) {
return importDeclaration(t,
keyChecker(t, declaration.id.properties, defaultTypeChecker),
keyChecker(t, declaration.id.properties, nonDefaultTypeChecker),
declaration.init.arguments[0])
} else if (isTransformableMemberExpression(path, declaration) && hasDefaultProperty(declaration.init)) {
return importDeclaration(t, [declaration.id], [], declaration.init.object.arguments[0]);
} else if (declarationsArr.length > 1) {
return variableDeclaration(t, path.node.kind, declaration);
} else {
return path.node;
}
})
path.replaceWithMultiple(newPaths);
}
}
};
};
var importDeclaration = function(babelTypes, defaultIdentifiers, generalIdentifiers, source) {
var defaultImports = defaultIdentifiers.map(function(iden) {
return babelTypes.importDefaultSpecifier(iden);
});
var generalImports = generalIdentifiers.map(function(iden) {
return babelTypes.importSpecifier(iden, iden);
});
return babelTypes.importDeclaration(
defaultImports.concat(generalImports),
source
);
}
var defaultTypeChecker = function(name) {
return name === 'default';
}
var nonDefaultTypeChecker = function(name) {
return name !== 'default';
}
var keyChecker = function(babelTypes, props, typeChecker) {
return props.filter(function(prop) {
return babelTypes.isIdentifier(prop.key) && typeChecker(prop.key.name);
}).map(function(prop) {
return prop.value;
});
}
var variableDeclaration = function(babelTypes, kind, declaration) {
return babelTypes.variableDeclaration(kind, [declaration])
}
var isInitialized = function(declaration) {
return declaration.init !== null;
}
var isFunctionCall = function(node) {
return node.type === 'CallExpression';
}
var isMemberExpression = function(node) {
return node.type === 'MemberExpression';
}
var isRequireCall = function(node) {
return node.callee.name === 'require';
}
var hasOnlyOneArgument = function(node) {
return node.arguments.length === 1;
}
var hasStringArguments = function(node) {
return node.arguments.every(function(argument) {
return argument.type === 'StringLiteral';
});
}
var isChildOfProgram = function(path) {
return path.parent.type === 'Program';
}
var isTransformableFunctionCall = function(path, node) {
return isChildOfProgram(path)
&& isInitialized(node)
&& isFunctionCall(node.init)
&& isRequireCall(node.init)
&& hasOnlyOneArgument(node.init)
&& hasStringArguments(node.init);
}
var isTransformableMemberExpression = function(path, node) {
return isChildOfProgram(path)
&& isInitialized(node)
&& isMemberExpression(node.init)
&& isFunctionCall(node.init.object)
&& isRequireCall(node.init.object)
&& hasOnlyOneArgument(node.init.object)
&& hasStringArguments(node.init.object);
}
var hasDefaultProperty = function(node) {
return node.property.name === 'default';
}
@nene
Copy link
Author

nene commented Mar 20, 2017

First off I suggest using ES6 syntax. That's kind of eating your own dogwood. Doesn't make sense to stick to ES5 nowadays.

What next catches my eye is some inconsistencies in indentation. Usually the code has 2 space indentation, but at times it uses some more. I'd suggest using ESLint to check for such style-inconsistencies. For example in here a block is indented by more then 2 spaces:

  return babelTypes.importDeclaration(
          defaultImports.concat(generalImports),
          source
        );

While here the return seems to not be connected with the lines that follow (I would expect the other lines to be indented):

  return isChildOfProgram(path) 
  && isInitialized(node) 
  && isFunctionCall(node.init) 
  && isRequireCall(node.init) 
  && hasOnlyOneArgument(node.init) 
  && hasStringArguments(node.init);

Naming is always critical:

  var t = babel.types;

Here we call it t, later we call it babelTypes. Should pick one name and stick to it. For my taste t feels too short. Though it is something you need pretty often, so you don't want to make it too long either. I'd say just calling it types would be a good middle-ground.

    VariableDeclaration: function(path, state) {
        var newPaths = path.node.declarations.map(function(declaration, index, declarationsArr) {
            ...
            } else if (declarationsArr.length > 1) {
              return variableDeclaration(t, path.node.kind, declaration);
            } else {
              return path.node;
            }
          })

I find it hard to understand what's going on in here.

First off there's the confusing nature of VariableDeclaration, which although singular in name, actually contains multiple declarations. The AST actually defines these inner declarations as type VariableDeclarator. I'd suggest using the same name in here: path.node.declarations.map(function(declarator, ...

I personally find that I never really need to use the third argument of Array.map() - I think it's clearer to just use the path.node.declarations in here as well, without giving it some additional name (declarationsArr) on the way.

But the use of path.node.declarations > 1 inside the loop is still pretty confusing. I'd expect to see such checks outside the loop. Further more, currently this code will split all multi-VariableDeclarations into single-VariableDeclarations. I'd expect some sort of check before doing any transforms:

if (!node.path.declarations.some(canBeTransformed)) {
    // do nothing
    return;
}

I should also mention that variableDeclaration() helper looks useless. Could just use t.variableDeclaration() directly.

Some functions take lots of arguments:

              return importDeclaration(t, 
                keyChecker(t, declaration.id.properties, defaultTypeChecker), 
                keyChecker(t, declaration.id.properties, nonDefaultTypeChecker), 
                declaration.init.arguments[0])

I'd say 3 arguments is usually too much already. Instead of positional arguments, consider using object to create named arguments:

              return importDeclaration({
                types: t, 
                defaultIdentifiers: keyChecker(t, declaration.id.properties, defaultTypeChecker), 
                generalIdentifiers: keyChecker(t, declaration.id.properties, nonDefaultTypeChecker), 
                source: declaration.init.arguments[0],
              });

It's weird though to see an array of defaultIdentifiers. ES6 allows just one default import/export. But I wouldn't worry about that too much - it's often simpler to work with arrays.

I'm more confused over the name generalIdentifiers. ES6 talks about "default" and "named" exports. I think it would be better to use the same terminology in here.

The name keyChecker is also confusing. I understand that it checks something about keys, but what? To make matters more confusing it takes argument called typeChecker which it uses to check prop.key.name ...so isn't it more of a name checker? And what does it return? typeChecker functions return boolean. I'd expect keyChecker to also return a boolean, but no, it actually returns an array of values.

I suggest extracting just the filter predicate to a separate function. The map-part is simple enough to be better expressed with some inline code. Written like this, it should be more obvious what kind of values these expressions return:

defaultIdentifiers: declaration.id.properties.filter(isDefaultProperty).map(prop => prop.value),
generalIdentifiers: declaration.id.properties.filter(isNamedProperty).map(prop => prop.value),

I like that you have divided up the code to various little helpers. However that's a pretty painful way to look for patterns in AST:

return isChildOfProgram(path) 
  && isInitialized(node) 
  && isMemberExpression(node.init)
  && isFunctionCall(node.init.object)
  && isRequireCall(node.init.object)
  && hasOnlyOneArgument(node.init.object)
  && hasStringArguments(node.init.object);

I used to write lots of similar code in Lebab, but that ended up being pretty difficult to understand and reason about.

So, in Lebab I wrote an utility to help with AST pattern-matching. With the help of that I could express the above code with just this:

return isChildOfProgram(path) &&
  isAstMatch(node, {
    init: {
      type: 'MemberExpression',
      object: {
        type: 'CallExpression',
        callee: {
          name: 'require'
        },
        arguments: matchesLength([
          {type: 'StringLiteral'}
        ]),
      }
    }
  });
}

I haven't extracted that utility from Lebab into a separate library. Might be a good opportunity to do so. Using it should also allow reusing bunch of code already existing in Lebab that uses this syntax.

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