From 592b2f888951fd6c7947553c3367775b9915ff83 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Thu, 1 Oct 2020 12:42:26 -0700
Subject: [PATCH] [BUGFIX lts] Allow computeds to have cycles in their deps

The bugfix for disallowed cycles in tracked props in #19138 attempted
to also narrow the number of cycles that are allowed in general. Cycles
should only be allowed for computed property deps, for legacy support
reasons. The logic to allow cycles for these tags in particular was
mistakenly added to `setup`, which runs on the _prototype_ of the class.
This meant that _instance_ computed props were not allowed to have
cycles, and this was causing failures in the ecosystem.

Added a test that failed and is fixed with this change. I also attempted
to create a cycle with `@alias` since it uses a different
implementation, but I wasn't able to create one which didn't result in
a Maximum Callstack style recursion error, so I think it's just not
possible at all anyways, since `@alias` is eager always and never
caches.
---
 packages/@ember/-internals/metal/lib/alias.ts |  2 ++
 .../@ember/-internals/metal/lib/computed.ts   | 12 ++++++----
 .../tests/system/object/computed_test.js      | 24 +++++++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/packages/@ember/-internals/metal/lib/alias.ts b/packages/@ember/-internals/metal/lib/alias.ts
index e7b62f189b1..204adf7766e 100644
--- a/packages/@ember/-internals/metal/lib/alias.ts
+++ b/packages/@ember/-internals/metal/lib/alias.ts
@@ -2,7 +2,9 @@ import { Meta, meta as metaFor } from '@ember/-internals/meta';
 import { inspect } from '@ember/-internals/utils';
 import { assert } from '@ember/debug';
 import EmberError from '@ember/error';
+import { DEBUG } from '@glimmer/env';
 import {
+  ALLOW_CYCLES,
   consumeTag,
   tagFor,
   tagMetaFor,
diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts
index f5850f23474..bbbe2af1f6e 100644
--- a/packages/@ember/-internals/metal/lib/computed.ts
+++ b/packages/@ember/-internals/metal/lib/computed.ts
@@ -323,10 +323,6 @@ export class ComputedProperty extends ComputedDescriptor {
       )
     );
 
-    if (DEBUG) {
-      ALLOW_CYCLES!.set(tagFor(obj, keyName), true);
-    }
-
     if (this._hasConfig === false) {
       assert(
         `Attempted to use @computed on ${keyName}, but it did not have a getter or a setter. You must either pass a get a function or getter/setter to @computed directly (e.g. \`@computed({ get() { ... } })\`) or apply @computed directly to a getter/setter`,
@@ -408,6 +404,10 @@ export class ComputedProperty extends ComputedDescriptor {
 
       if (_dependentKeys !== undefined) {
         updateTag(propertyTag!, getChainTagsForKeys(obj, _dependentKeys, tagMeta, meta));
+
+        if (DEBUG) {
+          ALLOW_CYCLES!.set(propertyTag, true);
+        }
       }
 
       meta.setValueFor(keyName, ret);
@@ -483,6 +483,10 @@ export class ComputedProperty extends ComputedDescriptor {
 
       if (_dependentKeys !== undefined) {
         updateTag(propertyTag, getChainTagsForKeys(obj, _dependentKeys, tagMeta, meta));
+
+        if (DEBUG) {
+          ALLOW_CYCLES!.set(propertyTag, true);
+        }
       }
 
       meta.setRevisionFor(keyName, valueForTag(propertyTag));
diff --git a/packages/@ember/-internals/runtime/tests/system/object/computed_test.js b/packages/@ember/-internals/runtime/tests/system/object/computed_test.js
index 40078e3fe59..369dcc40261 100644
--- a/packages/@ember/-internals/runtime/tests/system/object/computed_test.js
+++ b/packages/@ember/-internals/runtime/tests/system/object/computed_test.js
@@ -6,6 +6,7 @@ import {
   getWithDefault,
   observer,
   defineProperty,
+  notifyPropertyChange,
 } from '@ember/-internals/metal';
 import { oneWay as reads } from '@ember/object/computed';
 import { A as EmberArray, isArray } from '../../..';
@@ -540,5 +541,28 @@ moduleFor(
 
       assert.ok(true);
     }
+
+    ['@test computeds can have cycles'](assert) {
+      class CycleObject {
+        // eslint-disable-next-line getter-return
+        @computed('bar')
+        get foo() {}
+
+        // eslint-disable-next-line getter-return
+        @computed('foo')
+        get bar() {}
+      }
+
+      let obj = new CycleObject();
+
+      obj.bar;
+      obj.foo;
+
+      notifyPropertyChange(obj, 'bar');
+
+      obj.foo;
+
+      assert.ok(true);
+    }
   }
 );