Merge pull request #42 from ember-codemods/suchita/trackedFix

suchitadoshi1987 · web-flow · commit 7637631093c5 · 2020-03-29T22:25:41.000-07:00
Fix the removal of computed decorators
diff --git a/transforms/tracked-properties/README.md b/transforms/tracked-properties/README.md
@@ -141,7 +141,7 @@ export default class Foo extends Component {
     return `Name: ${get(this, 'firstName')} ${get(this, 'lastName')}`;
   }
 
-  @computed('areaCode')
+  @computed('areaCode', 'phone')
   get phoneNumber() {
     return `(${get(this, 'areaCode')}) ${get(this, 'phone')}`;
   }
@@ -190,15 +190,17 @@ export default class Foo extends Component {
   @tracked foo = 'bar';
   baz;
 
+  @computed('foo')
   get fooBar() {
     return `Foo: ${get(this, 'foo')}`;
   }
 
+  @computed('fooBar')
   get fooBarDetail() {
     return `Foo bar detail: ${get(this, 'fooBar')}`;
   }
 
-  @computed('bang')
+  @computed('fooBarDetail', 'bang')
   get fooBarDetailWithBaz() {
     return `(${get(this, 'fooBarDetail')}) ${get(this, 'baz')}`;
   }
@@ -252,7 +254,7 @@ export default class AddTeamComponent extends Component {
   @tracked teamName;
   @tracked noOfHackers;
 
-  @computed('fooBar')
+  @computed('fooBar', 'noOfHackers')
   get isMaxExceeded() {
     return this.noOfHackers > 10;
   }
@@ -316,12 +318,12 @@ export default class Foo extends Component {
   @alias('model.isFoo')
   isFoo;
 
-  @computed('isFoo')
+  @computed('baz', 'isFoo')
   get bazInfo() {
     return get(this, 'isFoo') ? `Name: ${get(this, 'baz')}` : 'Baz';
   }
 
-  @(computed('isFoo').readOnly())
+  @computed('bar', 'isFoo').readOnly()
   get barInfo() {
     return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
   }
@@ -385,7 +387,7 @@ export default class Foo extends Component {
     return `Bar: ${get(this, 'bar')}, Baz: ${get(this, 'baz')}`;
   }
 
-  @(computed('isFoo').readOnly())
+  @(computed('bar', 'isFoo').readOnly())
   get barInfo() {
     return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
   }
diff --git a/transforms/tracked-properties/__testfixtures__/chained-complex-computed.output.js b/transforms/tracked-properties/__testfixtures__/chained-complex-computed.output.js
@@ -8,7 +8,7 @@ export default class AddTeamComponent extends Component {
   @tracked teamName;
   @tracked noOfHackers;
 
-  @computed('fooBar')
+  @computed('fooBar', 'noOfHackers')
   get isMaxExceeded() {
     return this.noOfHackers > 10;
   }
diff --git a/transforms/tracked-properties/__testfixtures__/chained-computed.output.js b/transforms/tracked-properties/__testfixtures__/chained-computed.output.js
@@ -6,15 +6,17 @@ export default class Foo extends Component {
   @tracked foo = 'bar';
   baz;
 
+  @computed('foo')
   get fooBar() {
     return `Foo: ${get(this, 'foo')}`;
   }
 
+  @computed('fooBar')
   get fooBarDetail() {
     return `Foo bar detail: ${get(this, 'fooBar')}`;
   }
 
-  @computed('bang')
+  @computed('fooBarDetail', 'bang')
   get fooBarDetailWithBaz() {
     return `(${get(this, 'fooBarDetail')}) ${get(this, 'baz')}`;
   }
diff --git a/transforms/tracked-properties/__testfixtures__/complex.output.js b/transforms/tracked-properties/__testfixtures__/complex.output.js
@@ -12,7 +12,7 @@ export default class Foo extends Component {
     return `Name: ${get(this, 'firstName')} ${get(this, 'lastName')}`;
   }
 
-  @computed('areaCode')
+  @computed('areaCode', 'phone')
   get phoneNumber() {
     return `(${get(this, 'areaCode')}) ${get(this, 'phone')}`;
   }
diff --git a/transforms/tracked-properties/__testfixtures__/non-computed-decorators.output.js b/transforms/tracked-properties/__testfixtures__/non-computed-decorators.output.js
@@ -11,12 +11,12 @@ export default class Foo extends Component {
   @alias('model.isFoo')
   isFoo;
 
-  @computed('isFoo')
+  @computed('baz', 'isFoo')
   get bazInfo() {
     return get(this, 'isFoo') ? `Name: ${get(this, 'baz')}` : 'Baz';
   }
 
-  @computed('isFoo').readOnly()
+  @computed('bar', 'isFoo').readOnly()
   get barInfo() {
     return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
   }
diff --git a/transforms/tracked-properties/__testfixtures__/read-only-computed-decorators.output.js b/transforms/tracked-properties/__testfixtures__/read-only-computed-decorators.output.js
@@ -15,7 +15,7 @@ export default class Foo extends Component {
     return `Bar: ${get(this, 'bar')}, Baz: ${get(this, 'baz')}`;
   }
 
-  @(computed('isFoo').readOnly())
+  @(computed('bar', 'isFoo').readOnly())
   get barInfo() {
     return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
   }
diff --git a/transforms/tracked-properties/index.js b/transforms/tracked-properties/index.js
@@ -119,38 +119,49 @@ module.exports = function transformer(file, api) {
     trackedConvertedSource = reformatTrackedDecorators(trackedConvertedSource);
   }
 
-  // Iterate on all the `computed` decorators and for each node, check if
-  // all the arguments are a part of the `classProps` array, if so, then
-  // remove the `@computed` decorator, else remove the arguments that are
-  // defined locally in the class.
+  /**
+   * Iterate on all the class items, if the class item has decorators and it is not a dependent
+   * key of some other property, then go ahead and check if the `computed` decorator can be safely removed.
+   */
   const convertedResult = j(trackedConvertedSource)
-    .find(j.Decorator)
-    .filter(path => {
-      return (
-        path.node.expression.type === 'CallExpression' &&
-        _isComputedProperty(path.node)
-      );
-    })
+    .find(j.ClassBody)
     .forEach(path => {
-      const isReadOnlyProperty = _isReadOnlyComputedProperty(path.node);
-      const computedPropArguments = isReadOnlyProperty
-        ? path.node.expression.callee.object.arguments
-        : path.node.expression.arguments;
+      path.node.body.forEach(classItem => {
+        const propName = classItem.key.name;
+        // Check if the class item is not a dependent key of any other computed properties in the class
+        // and if the item has any decorators.
+        if (
+          !Object.keys(computedPropsMap).some(item =>
+            computedPropsMap[item].includes(propName)
+          ) &&
+          classItem.decorators
+        ) {
+          classItem.decorators.forEach((decoratorItem, i) => {
+            if (
+              decoratorItem.expression.type === 'CallExpression' &&
+              _isComputedProperty(decoratorItem)
+            ) {
+              const isReadOnlyProperty = _isReadOnlyComputedProperty(
+                decoratorItem
+              );
+              const computedPropArguments = isReadOnlyProperty
+                ? decoratorItem.expression.callee.object.arguments
+                : decoratorItem.expression.arguments;
 
-      const dependentKeys = getDependentKeys(
-        computedPropArguments,
-        computedPropsMap,
-        classProps
-      );
-      if (!dependentKeys.length) {
-        path.replace();
-      } else {
-        if (isReadOnlyProperty) {
-          path.node.expression.callee.object.arguments = dependentKeys;
-        } else {
-          path.node.expression.arguments = dependentKeys;
+              const dependentKeys = getDependentKeys(
+                computedPropArguments,
+                computedPropsMap,
+                classProps
+              );
+              // If all the arguments of the decorator are class properties, then remove the decorator completely
+              // from the item.
+              if (!dependentKeys.length) {
+                classItem.decorators.splice(i, 1);
+              }
+            }
+          });
         }
-      }
+      });
     });
 
   return shouldImportBeAdded

Original file line number	Diff line number	Diff line change
`@@ -141,7 +141,7 @@ export default class Foo extends Component {`
`141`	`141`	return `Name: ${get(this, 'firstName')} ${get(this, 'lastName')}`;
`142`	`142`	`}`
`143`	`143`
`144`		`- @computed('areaCode')`
	`144`	`+ @computed('areaCode', 'phone')`
`145`	`145`	`get phoneNumber() {`
`146`	`146`	return `(${get(this, 'areaCode')}) ${get(this, 'phone')}`;
`147`	`147`	`}`
`@@ -190,15 +190,17 @@ export default class Foo extends Component {`
`190`	`190`	`@tracked foo = 'bar';`
`191`	`191`	`baz;`
`192`	`192`
	`193`	`+ @computed('foo')`
`193`	`194`	`get fooBar() {`
`194`	`195`	return `Foo: ${get(this, 'foo')}`;
`195`	`196`	`}`
`196`	`197`
	`198`	`+ @computed('fooBar')`
`197`	`199`	`get fooBarDetail() {`
`198`	`200`	return `Foo bar detail: ${get(this, 'fooBar')}`;
`199`	`201`	`}`
`200`	`202`
`201`		`- @computed('bang')`
	`203`	`+ @computed('fooBarDetail', 'bang')`
`202`	`204`	`get fooBarDetailWithBaz() {`
`203`	`205`	return `(${get(this, 'fooBarDetail')}) ${get(this, 'baz')}`;
`204`	`206`	`}`
`@@ -252,7 +254,7 @@ export default class AddTeamComponent extends Component {`
`252`	`254`	`@tracked teamName;`
`253`	`255`	`@tracked noOfHackers;`
`254`	`256`
`255`		`- @computed('fooBar')`
	`257`	`+ @computed('fooBar', 'noOfHackers')`
`256`	`258`	`get isMaxExceeded() {`
`257`	`259`	`return this.noOfHackers > 10;`
`258`	`260`	`}`
`@@ -316,12 +318,12 @@ export default class Foo extends Component {`
`316`	`318`	`@alias('model.isFoo')`
`317`	`319`	`isFoo;`
`318`	`320`
`319`		`- @computed('isFoo')`
	`321`	`+ @computed('baz', 'isFoo')`
`320`	`322`	`get bazInfo() {`
`321`	`323`	return get(this, 'isFoo') ? `Name: ${get(this, 'baz')}` : 'Baz';
`322`	`324`	`}`
`323`	`325`
`324`		`- @(computed('isFoo').readOnly())`
	`326`	`+ @computed('bar', 'isFoo').readOnly()`
`325`	`327`	`get barInfo() {`
`326`	`328`	return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
`327`	`329`	`}`
`@@ -385,7 +387,7 @@ export default class Foo extends Component {`
`385`	`387`	return `Bar: ${get(this, 'bar')}, Baz: ${get(this, 'baz')}`;
`386`	`388`	`}`
`387`	`389`
`388`		`- @(computed('isFoo').readOnly())`
	`390`	`+ @(computed('bar', 'isFoo').readOnly())`
`389`	`391`	`get barInfo() {`
`390`	`392`	return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
`391`	`393`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ export default class AddTeamComponent extends Component {`
`8`	`8`	`@tracked teamName;`
`9`	`9`	`@tracked noOfHackers;`
`10`	`10`
`11`		`- @computed('fooBar')`
	`11`	`+ @computed('fooBar', 'noOfHackers')`
`12`	`12`	`get isMaxExceeded() {`
`13`	`13`	`return this.noOfHackers > 10;`
`14`	`14`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,15 +6,17 @@ export default class Foo extends Component {`
`6`	`6`	`@tracked foo = 'bar';`
`7`	`7`	`baz;`
`8`	`8`
	`9`	`+ @computed('foo')`
`9`	`10`	`get fooBar() {`
`10`	`11`	return `Foo: ${get(this, 'foo')}`;
`11`	`12`	`}`
`12`	`13`
	`14`	`+ @computed('fooBar')`
`13`	`15`	`get fooBarDetail() {`
`14`	`16`	return `Foo bar detail: ${get(this, 'fooBar')}`;
`15`	`17`	`}`
`16`	`18`
`17`		`- @computed('bang')`
	`19`	`+ @computed('fooBarDetail', 'bang')`
`18`	`20`	`get fooBarDetailWithBaz() {`
`19`	`21`	return `(${get(this, 'fooBarDetail')}) ${get(this, 'baz')}`;
`20`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ export default class Foo extends Component {`
`12`	`12`	return `Name: ${get(this, 'firstName')} ${get(this, 'lastName')}`;
`13`	`13`	`}`
`14`	`14`
`15`		`- @computed('areaCode')`
	`15`	`+ @computed('areaCode', 'phone')`
`16`	`16`	`get phoneNumber() {`
`17`	`17`	return `(${get(this, 'areaCode')}) ${get(this, 'phone')}`;
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,12 +11,12 @@ export default class Foo extends Component {`
`11`	`11`	`@alias('model.isFoo')`
`12`	`12`	`isFoo;`
`13`	`13`
`14`		`- @computed('isFoo')`
	`14`	`+ @computed('baz', 'isFoo')`
`15`	`15`	`get bazInfo() {`
`16`	`16`	return get(this, 'isFoo') ? `Name: ${get(this, 'baz')}` : 'Baz';
`17`	`17`	`}`
`18`	`18`
`19`		`- @computed('isFoo').readOnly()`
	`19`	`+ @computed('bar', 'isFoo').readOnly()`
`20`	`20`	`get barInfo() {`
`21`	`21`	return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ export default class Foo extends Component {`
`15`	`15`	return `Bar: ${get(this, 'bar')}, Baz: ${get(this, 'baz')}`;
`16`	`16`	`}`
`17`	`17`
`18`		`- @(computed('isFoo').readOnly())`
	`18`	`+ @(computed('bar', 'isFoo').readOnly())`
`19`	`19`	`get barInfo() {`
`20`	`20`	return get(this, 'isFoo') ? `Name: ${get(this, 'bar')}` : 'Bar';
`21`	`21`	`}`