Merge pull request #143 from canjs/fix-comments-removed

Comments should not be removed
canjs · Sep 30, 2019 · d904844 · d904844
2 parents 8ffdafc + abbc518
commit d904844
Show file tree

Hide file tree

Showing 22 changed files with 155 additions and 32 deletions.
diff --git a/src/templates/can-observable-array/observable-array-input.html b/src/templates/can-observable-array/observable-array-input.html
@@ -1,4 +1,8 @@
 <script>
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 const MyOtherList = DefineList.extend({
   '#': 'string',
 
@@ -9,6 +13,7 @@
 </script>
 
 <script>
+// Foo bar
 Messages.List = DefineList.extend({
   '#': 'string',
 
@@ -52,4 +57,4 @@
 class UserList extends StacheElement {
   static get props () {}
 }
-</script>
+</script>
diff --git a/src/templates/can-observable-array/observable-array-input.js b/src/templates/can-observable-array/observable-array-input.js
@@ -1,5 +1,10 @@
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 const MyList = DefineList.extend({ '#': 'string' });
 
+// A comment
 const MyOtherList = DefineList.extend({
   '#': 'string',
 

diff --git a/src/templates/can-observable-array/observable-array-output.html b/src/templates/can-observable-array/observable-array-output.html
@@ -1,4 +1,8 @@
 <script>
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 class MyOtherList extends ObservableArray {
   static get props() {
     return {
@@ -13,6 +17,7 @@
 </script>
 
 <script>
+// Foo bar
 class MessagesList extends ObservableArray {
   static get props() {
     return {
@@ -68,4 +73,4 @@
 class UserList extends StacheElement {
   static get props () {}
 }
-</script>
+</script>
diff --git a/src/templates/can-observable-array/observable-array-output.js b/src/templates/can-observable-array/observable-array-output.js
@@ -1,9 +1,14 @@
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 class MyList extends ObservableArray {
   static get props() {
     return { '#': 'string' };
   }
 }
 
+// A comment
 class MyOtherList extends ObservableArray {
   static get props() {
     return {

diff --git a/src/templates/can-observable-object/observable-object-input.js b/src/templates/can-observable-object/observable-object-input.js
@@ -1,3 +1,4 @@
+// Comment line should not be removed
 const Foo = DefineMap.extend({
   name: 'string',
   completed: {
@@ -6,6 +7,11 @@ const Foo = DefineMap.extend({
   }
 });
 
+/**
+ * Comment Block should not be removed
+ * Foo bar
+ */
+//foo
 const VM = DefineMap.extend({
   firstName: 'string',
   lastName: 'string',

diff --git a/src/templates/can-observable-object/observable-object-input.md b/src/templates/can-observable-object/observable-object-input.md
@@ -1,4 +1,5 @@
 ```js
+// Comment line should not be removed
 const Foo = DefineMap.extend({
   name: 'string',
   completed: {
@@ -9,6 +10,11 @@ const Foo = DefineMap.extend({
 ```
 
 ```js
+/**
+ * Comment Block should not be removed
+ * Foo bar
+ */
+//foo
 const VM = DefineMap.extend({
   firstName: 'string',
   lastName: 'string',
@@ -54,4 +60,4 @@ export default DefineMap.extend('MyMap', {
     return `${firstName} ${lastName}`
   }
 });
-```
+```
diff --git a/src/templates/can-observable-object/observable-object-output.js b/src/templates/can-observable-object/observable-object-output.js
@@ -1,3 +1,4 @@
+// Comment line should not be removed
 class Foo extends ObservableObject {
   static get props() {
     return {
@@ -10,6 +11,11 @@ class Foo extends ObservableObject {
   }
 }
 
+/**
+ * Comment Block should not be removed
+ * Foo bar
+ */
+//foo
 class VM extends ObservableObject {
   static get props() {
     return {

diff --git a/src/templates/can-observable-object/observable-object-output.md b/src/templates/can-observable-object/observable-object-output.md
@@ -1,4 +1,5 @@
 ```js
+// Comment line should not be removed
 class Foo extends ObservableObject {
   static get props() {
     return {
@@ -13,6 +14,11 @@ class Foo extends ObservableObject {
 ```
 
 ```js
+/**
+ * Comment Block should not be removed
+ * Foo bar
+ */
+//foo
 class VM extends ObservableObject {
   static get props() {
     return {
@@ -70,4 +76,4 @@ export default class MyMap extends ObservableObject {
     };
   }
 }
-```
+```
diff --git a/src/templates/can-property-definitions/property-functions-input.js b/src/templates/can-property-definitions/property-functions-input.js
@@ -6,6 +6,7 @@ class Bar extends ObservableObject {
         return new Item(args);
       },
       get sayHi () {
+        // Comment inside getter
         return `Hello ${this.name}`;
       },
       set sayHi (val) {
@@ -18,6 +19,7 @@ class Bar extends ObservableObject {
 class Foo extends ObservableObject {
   static get props() {
     return {
+      // The comment should not be removed
       async createItem(args) {
         return new Item(args);
       }

diff --git a/src/templates/can-property-definitions/property-functions-output.js b/src/templates/can-property-definitions/property-functions-output.js
@@ -4,6 +4,7 @@ class Bar extends ObservableObject {
       name: 'string',
 
       get sayHi () {
+        // Comment inside getter
         return `Hello ${this.name}`;
       },
 
@@ -23,6 +24,7 @@ class Foo extends ObservableObject {
     return {};
   }
 
+  // The comment should not be removed
   async createItem(args) {
     return new Item(args);
   }

diff --git a/src/templates/can-property-definitions/property-functions.js b/src/templates/can-property-definitions/property-functions.js
@@ -5,21 +5,36 @@ import fileTransform from '../../../utils/fileUtil';
 function transformer(file, api) {
   const debug = makeDebug(`can-migrate:can-property-definitions/property-functions:${file.path}`);
   const j = api.jscodeshift;
-  
+
   return fileTransform(file, function (source) {
     const root = j(source);
-    
+
     return find(root, 'FunctionExpression', function (props, rootPath) {
       const rootDefine = rootPath.value.value.body.body[0].argument.properties;
 
       return props.forEach(prop => {
         if (prop.kind !== 'get' && prop.kind !== 'set') {
+
           // Add the method to the class body
-          rootPath.parent.value.body.push(j.methodDefinition(
+          const method = j.methodDefinition(
             'method',
             prop.key,
             prop.value
-          ));
+          );
+
+          // Keep the comments where they belong
+          if (prop.leadingComments && prop.leadingComments.length) {
+            method.comments = method.comments ? method.comments : [];
+            prop.leadingComments.forEach(function(comment){
+              if (comment.type === 'CommentLine') {
+                method.comments.push(j.commentLine(comment.value, true, false));
+              }
+              if (comment.type === 'CommentBlock') {
+                method.comments.push(j.commentBlock(comment.value, true, false));
+              }
+            });
+          }
+          rootPath.parent.value.body.push(method);
 
           debug(`Removing ${prop.key} from define () {} into class definition`);
 
@@ -38,4 +53,4 @@ function transformer(file, api) {
 
 transformer.forceJavaScriptTransform = true;
 
-export default transformer;
+export default transformer;
diff --git a/src/utils/defineTransform.js b/src/utils/defineTransform.js
@@ -69,7 +69,7 @@ export default function defineTransform ({
         .find(j.Identifier, {
           name: varDeclaration
         });
-      
+
       varDeclaration = nameInUse.length > 0 ? `${varDeclaration}Model` : varDeclaration;
 
       // Update refs if we are of type AssignmentExpression
@@ -78,31 +78,43 @@ export default function defineTransform ({
 
     debug(`Replacing ${varDeclaration} with ${extendedClassName} class`);
 
-    j(classPath).replaceWith(
-      createClass({
-        j,
-        className: varDeclaration,
-        body: [
-          createMethod({
-            j,
-            method: false, // Want this to be a getter
-            name: 'props',
-            blockStatement: [j.returnStatement(propDefinitionsArg)],
-            isStatic: true
-          })
-        ],
-        extendedClassName
-      })
-    );
+    const classDeclaration = createClass({
+      j,
+      className: varDeclaration,
+      body: [
+        createMethod({
+          j,
+          method: false, // Want this to be a getter
+          name: 'props',
+          blockStatement: [j.returnStatement(propDefinitionsArg)],
+          isStatic: true
+        })
+      ],
+      extendedClassName
+    });
+
+    if (classPath.node.comments && classPath.node.comments.length) {
+      classDeclaration.comments = classDeclaration.comments ? classDeclaration.comments : [];
+      classPath.node.comments.forEach(function(comment){
+        if (comment.type === 'CommentLine') {
+          classDeclaration.comments.push(j.commentLine(comment.value, true, false));
+        }
+        if (comment.type === 'CommentBlock') {
+          classDeclaration.comments.push(j.commentBlock(comment.value, true, false));
+        }
+      });
+    }
+
+    j(classPath).replaceWith(classDeclaration);
   });
-  
+
   // If we have changed the ref we need to update all references
   updateRefs.forEach(ref => {
     replaceRefs(j, root, {
       oldLocalName: ref,
       newLocalName: ref.varDeclaration
     });
   });
-  
+
   return root.toSource();
 }
diff --git a/test/fixtures/version-6/can-observable-array/observable-array-input.html b/test/fixtures/version-6/can-observable-array/observable-array-input.html
@@ -1,4 +1,8 @@
 <script>
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 const MyOtherList = DefineList.extend({
   '#': 'string',
 
@@ -9,6 +13,7 @@
 </script>
 
 <script>
+// Foo bar
 Messages.List = DefineList.extend({
   '#': 'string',
 
@@ -52,4 +57,4 @@
 class UserList extends StacheElement {
   static get props () {}
 }
-</script>
+</script>
diff --git a/test/fixtures/version-6/can-observable-array/observable-array-input.js b/test/fixtures/version-6/can-observable-array/observable-array-input.js
@@ -1,5 +1,10 @@
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 const MyList = DefineList.extend({ '#': 'string' });
 
+// A comment
 const MyOtherList = DefineList.extend({
   '#': 'string',
 

diff --git a/test/fixtures/version-6/can-observable-array/observable-array-output.html b/test/fixtures/version-6/can-observable-array/observable-array-output.html
@@ -1,4 +1,8 @@
 <script>
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 class MyOtherList extends ObservableArray {
   static get props() {
     return {
@@ -13,6 +17,7 @@
 </script>
 
 <script>
+// Foo bar
 class MessagesList extends ObservableArray {
   static get props() {
     return {
@@ -68,4 +73,4 @@
 class UserList extends StacheElement {
   static get props () {}
 }
-</script>
+</script>
diff --git a/test/fixtures/version-6/can-observable-array/observable-array-output.js b/test/fixtures/version-6/can-observable-array/observable-array-output.js
@@ -1,9 +1,14 @@
+/**
+ * Multiline comment
+ * Foo Bar
+ */
 class MyList extends ObservableArray {
   static get props() {
     return { '#': 'string' };
   }
 }
 
+// A comment
 class MyOtherList extends ObservableArray {
   static get props() {
     return {

diff --git a/test/fixtures/version-6/can-observable-object/observable-object-input.js b/test/fixtures/version-6/can-observable-object/observable-object-input.js
@@ -1,3 +1,4 @@
+// Comment line should not be removed
 const Foo = DefineMap.extend({
   name: 'string',
   completed: {
@@ -6,6 +7,11 @@ const Foo = DefineMap.extend({
   }
 });
 
+/**
+ * Comment Block should not be removed
+ * Foo bar
+ */
+//foo
 const VM = DefineMap.extend({
   firstName: 'string',
   lastName: 'string',