Fix the removal of computed decorators

Author: Suchita Doshi

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';
   }

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;
   }

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')}`;
   }

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')}`;
   }

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';
   }

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';
   }

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

transforms/tracked-properties/utils/helper.js

@@ -63,7 +63,7 @@ function _doesContainNonLocalArgs(argItem, computedMap, classProperties) {
 
     // If currItem is not a class property and
     // if it is not a computed property with dependent keys, return true.
-    if (!classProperties.includes(currItem) && !dependentKeys) {
+    if ((!classProperties.includes(currItem) && !dependentKeys) || Object.keys(computedMap).includes(currItem)) {
       return true;
     }
     // If currItem itself is a computed property, then it would have dependent keys.