Improved Comment Support (fixes jamischarles#3)

test/fixtures/amd.after.js

@@ -1,6 +1,18 @@
+/**
+ * Top of the mornin' to ya
+ */
 console.log('a');
+
+/**
+ * Hunger Games is real
+ */
+
 console.log('b');
+
+// line comment a
 import 'a';
+
+// line comment b
 console.log('dont bother with a');
 import a from 'a';
 console.log('I need you A', a.isUsed());
@@ -12,5 +24,4 @@ import moduleA from 'a';
 import moduleB from 'b';
 import moduleC from 'c';
 import 'd';
-console.log('xyz');
-
+console.log('xyz');

test/fixtures/amd.before.js

@@ -1,12 +1,21 @@
+/**
+ * Top of the mornin' to ya
+ */
 define(function() {
 	console.log('a');
 });
 
+/**
+ * Hunger Games is real
+ */
+
 define([], function() {
 	console.log('b');
 });
 
+// line comment a
 define(['a'], function() {
+	// line comment b
 	console.log('dont bother with a');
 });
 

test/fixtures/cjs.after.js

@@ -1,3 +1,5 @@
+// CJS top comment
+
 import jamis from 'jquery';
 import $ from 'jquery';
 import $ from 'jquery';
@@ -13,4 +15,4 @@ import $ from 'jquery';
 var jamis = 'bar', bar, foo = 'bar';
 
 import {routeTo} from '../routeHelper';
-import {pluck as fetch} from '../someUtil';
+import {pluck as fetch} from '../someUtil';

test/fixtures/cjs.before.js

@@ -1,5 +1,7 @@
+// CJS top comment
+
 var jamis = require('jquery');
-var $ = require('jquery');
+var $ = require('jquery'); // end of line comment will get eaten
 var $ = require('jquery');
 require('something');
 

transforms/amd.js

@@ -6,33 +6,48 @@ var util = require('../utils/main');
 /**
  * Will convert require() statements in a js file to es6 import statements
  */
-module.exports = function(fileInfo, api) {
+module.exports = function(file, api) {
     var j = api.jscodeshift;
+    var root = j(file.source);
+    var leadingComment = root.find(j.Program).get('body', 0).node.leadingComments;
 
+    // root.find(j.VariableDeclaration).at(0).remove();
 
-    return j(fileInfo.source)
-    .find(j.CallExpression, { callee: { name: 'define' } }) // find require() function calls
-    .filter(function(p) { return p.parentPath.parentPath.name === 'body'; })
-    .forEach(function(p) {
-
-        // define(function() { });
-        if (p.value.arguments.length === 1) {
-            return j(p.parent).replaceWith(p.value.arguments[0].body.body);
-        }
-
-        // define(['a', 'b', 'c'], function(a, b, c) { });
-        if (p.value.arguments.length === 2) {
-            var props = p.value.arguments[0].elements;
-            var importStatements = props.map(function(prop, i) {
-                var moduleName = prop.value;
-                var variableName = p.value.arguments[1].params[i] && p.value.arguments[1].params[i].name;
-                return util.createImportStatement(moduleName, variableName);
-            });
-            Array.prototype.push.apply(importStatements, p.value.arguments[1].body.body);
-            return j(p.parent).replaceWith(importStatements);
-        }
-
-    })
-    // FIXME: make this a config to pass in?
-    .toSource({ quote: 'single' });
+    root
+        .find(j.CallExpression, { callee: { name: 'define' } }) // find require() function calls
+        .filter(function(p) { return p.parentPath.parentPath.name === 'body'; })
+        .forEach(function(p) {
+
+            // define(function() { });
+            if (p.value.arguments.length === 1) {
+                return j(p.parent).replaceWith(p.value.arguments[0].body.body);
+            }
+
+            // define(['a', 'b', 'c'], function(a, b, c) { });
+            if (p.value.arguments.length === 2) {
+                var props = p.value.arguments[0].elements;
+                var comments = p.parent.value.comments || [];
+                var importStatements = props.map(function(prop, i) {
+                    var moduleName = prop.value;
+                    var variableName = p.value.arguments[1].params[i] && p.value.arguments[1].params[i].name;
+                    return util.createImportStatement(moduleName, variableName);
+                });
+
+                // add the body after the import statements
+                Array.prototype.push.apply(importStatements, p.value.arguments[1].body.body);
+
+                // add any comments at the top
+                importStatements[0].comments = comments;
+
+                // done
+                return j(p.parent).replaceWith(importStatements);
+            }
+
+        });
+
+        // re-add comment to to the top
+        root.get().node.comments = leadingComment;
+
+        // FIXME: make this a config to pass in?
+        return root.toSource({ quote: 'single' });
 };

transforms/cjs.js

@@ -6,56 +6,63 @@ var util = require('../utils/main');
 /**
  * Will convert require() statements in a js file to es6 import statements
  */
-module.exports = function(fileInfo, api) {
+module.exports = function(file, api) {
     var j = api.jscodeshift;
-    return j(fileInfo.source)
-    .find(j.CallExpression, {callee: { name: 'require' }}) // find require() function calls
-    .forEach(function(p) {
+    var root = j(file.source);
+    var leadingComment = root.find(j.Program).get('body', 0).node.leadingComments;
 
-        var props, importStatement;
+    // migrate away all the imports
+    root.find(j.CallExpression, { callee: { name: 'require' } }) // find require() function calls
+        .forEach(function(p) {
 
-        // is this require() part of a var declaration?
-        // var $ = require('jquery');
-        var varParent = findVarParent(p);
+            var props, importStatement;
 
-        // am I part of a single var statement?
-        if (varParent && isSingleVar(varParent)) {
-            // wrap the variableDeclarator in a VariableDeclaration (for more consistent prop extraction)
-            var varStatement = j.variableDeclaration('var', [p.parentPath.value]);
-            props = util.getPropsFromRequire(varStatement);
+            // is this require() part of a var declaration?
+            // var $ = require('jquery');
+            var varParent = findVarParent(p);
 
-            importStatement = util.createImportStatement(props.moduleName, props.variableName, props.propName);
+            // am I part of a single var statement?
+            if (varParent && isSingleVar(varParent)) {
+                // wrap the variableDeclarator in a VariableDeclaration (for more consistent prop extraction)
+                var varStatement = j.variableDeclaration('var', [p.parentPath.value]);
+                props = util.getPropsFromRequire(varStatement);
 
-            // insert the new import statement AFTER the singleVar and and remove the require() from the single var.
-            //j(varParent).insertAfter(importStatement);
-            // HACK: Using before for now, to avoid it mangling the whitespace after the var statement.
-            // This will cause problems if the single var statement contains deps that the other els depend on
-            j(p.parentPath.parent).insertBefore(importStatement);
-            j(p.parent).remove();
+                importStatement = util.createImportStatement(props.moduleName, props.variableName, props.propName);
 
-            return;
+                // insert the new import statement AFTER the singleVar and and remove the require() from the single var.
+                //j(varParent).insertAfter(importStatement);
+                // HACK: Using before for now, to avoid it mangling the whitespace after the var statement.
+                // This will cause problems if the single var statement contains deps that the other els depend on
+                j(p.parentPath.parent).insertBefore(importStatement);
+                j(p.parent).remove();
+
+                return;
+
+            } else if (varParent) {
+                props = util.getPropsFromRequire(varParent);
+                importStatement = util.createImportStatement(props.moduleName, props.variableName, props.propName);
 
-        } else if (varParent) {
-            props = util.getPropsFromRequire(varParent);
+                // reach higher in the tree to replace the var statement with an import. Needed so we don't just
+                // replace require() with the import statement.
+                j(varParent).replaceWith(importStatement);
+                return;
+
+            }
+
+            // not part of a var statment
+            // require('underscore');
+            props = util.getPropsFromRequire(p.parent); // use.p.parent so it includes the semicolon
             importStatement = util.createImportStatement(props.moduleName, props.variableName, props.propName);
 
-            // reach higher in the tree to replace the var statement with an import. Needed so we don't just
-            // replace require() with the import statement.
-            j(varParent).replaceWith(importStatement);
+            j(p.parent).replaceWith(importStatement);
             return;
+        });
 
-        }
-
-        // not part of a var statment
-        // require('underscore');
-        props = util.getPropsFromRequire(p.parent); // use.p.parent so it includes the semicolon
-        importStatement = util.createImportStatement(props.moduleName, props.variableName, props.propName);
+    // re-add comment to to the top
+    root.get().node.comments = leadingComment;
 
-        j(p.parent).replaceWith(importStatement);
-        return;
-    })
     // FIXME: make this a config to pass in?
-    .toSource({quote: 'single'});
+    return root.toSource({ quote: 'single' });
 };