[FEATURE] Scopes down the mandatory setter

Scopes down the mandatory setter, like it was described in the [quest issue](#18769). It should only be applied when a value is being used as a dependency in a computed property or an observer.
emberjs · May 9, 2020 · d453ec9 · d453ec9
1 parent bd53ff2
commit d453ec9
Show file tree

Hide file tree

Showing 9 changed files with 59 additions and 32 deletions.
diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js
@@ -2788,16 +2788,6 @@ moduleFor(
 
       this.assertText('initial value - initial value');
 
-      if (DEBUG) {
-        let message = /You attempted to update .*, but it is being tracked by a tracking context/;
-
-        expectAssertion(() => {
-          component.bar = 'foo-bar';
-        }, message);
-
-        this.assertText('initial value - initial value');
-      }
-
       runTask(() => {
         component.set('bar', 'updated value');
       });

diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js
@@ -462,6 +462,37 @@ moduleFor(
 
       this.assertText('Kris Selden');
     }
+
+    '@test does not setup mandatory setter for untracked values'() {
+      let person;
+
+      class Person {
+        constructor(first, last) {
+          person = this;
+          this.first = first;
+          this.last = last;
+        }
+      }
+
+      class PersonComponent extends GlimmerishComponent {
+        person = new Person(this.args.first, this.args.last);
+      }
+
+      this.registerComponent('person-wrapper', {
+        ComponentClass: PersonComponent,
+        template: '{{this.person.first}} {{this.person.last}}',
+      });
+
+      this.render('<PersonWrapper @first={{first}} @last={{last}} />', {
+        first: 'robert',
+        last: 'jackson',
+      });
+
+      this.assertText('robert jackson');
+
+      // check to make sure we can still mutate the person
+      person.first = 'max';
+    }
   }
 );
 

diff --git a/packages/@ember/-internals/metal/lib/alias.ts b/packages/@ember/-internals/metal/lib/alias.ts
@@ -91,7 +91,7 @@ export class AliasedProperty extends ComputedDescriptor {
     let lastRevision = getLastRevisionFor(obj, keyName);
 
     if (!validateTag(propertyTag, lastRevision)) {
-      updateTag(propertyTag, combine(getChainTagsForKey(obj, this.altKey)));
+      updateTag(propertyTag, combine(getChainTagsForKey(obj, this.altKey, true)));
       setLastRevisionFor(obj, keyName, valueForTag(propertyTag));
       finishLazyChains(obj, keyName, ret);
     }

diff --git a/packages/@ember/-internals/metal/lib/chain-tags.ts b/packages/@ember/-internals/metal/lib/chain-tags.ts
@@ -38,17 +38,17 @@ export function finishLazyChains(obj: any, key: string, value: any) {
   }
 }
 
-export function getChainTagsForKeys(obj: any, keys: string[]) {
+export function getChainTagsForKeys(obj: any, keys: string[], addMandatorySetter = false) {
   let chainTags: Tag[] = [];
 
   for (let i = 0; i < keys.length; i++) {
-    chainTags.push(...getChainTagsForKey(obj, keys[i]));
+    chainTags.push(...getChainTagsForKey(obj, keys[i], addMandatorySetter));
   }
 
   return chainTags;
 }
 
-export function getChainTagsForKey(obj: any, path: string) {
+export function getChainTagsForKey(obj: any, path: string, addMandatorySetter = false) {
   let chainTags: Tag[] = [];
 
   let current: any = obj;
@@ -137,19 +137,19 @@ export function getChainTagsForKey(obj: any, path: string) {
             typeof item === 'object'
           );
 
-          chainTags.push(tagForProperty(item, segment));
+          chainTags.push(tagForProperty(item, segment, addMandatorySetter));
         }
       }
 
       // Push the tag for the array length itself
-      chainTags.push(tagForProperty(current, '[]'));
+      chainTags.push(tagForProperty(current, '[]', addMandatorySetter));
 
       break;
     }
 
     // TODO: Assert that current[segment] isn't an undecorated, non-MANDATORY_SETTER/dependentKeyCompat getter
 
-    let propertyTag = tagForProperty(current, segment);
+    let propertyTag = tagForProperty(current, segment, addMandatorySetter);
     descriptor = descriptorForProperty(current, segment);
 
     chainTags.push(propertyTag);

diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts
@@ -631,7 +631,7 @@ export class ComputedProperty extends ComputedDescriptor {
       }
 
       if (this._dependentKeys !== undefined) {
-        let tag = combine(getChainTagsForKeys(obj, this._dependentKeys));
+        let tag = combine(getChainTagsForKeys(obj, this._dependentKeys, true));
 
         upstreamTag = upstreamTag === undefined ? tag : combine([upstreamTag, tag]);
       }
@@ -683,7 +683,7 @@ export class ComputedProperty extends ComputedDescriptor {
       let propertyTag = tagForProperty(obj, keyName) as UpdatableTag;
 
       if (this._dependentKeys !== undefined) {
-        updateTag(propertyTag, combine(getChainTagsForKeys(obj, this._dependentKeys)));
+        updateTag(propertyTag, combine(getChainTagsForKeys(obj, this._dependentKeys, true)));
       }
 
       setLastRevisionFor(obj, keyName, valueForTag(propertyTag));

diff --git a/packages/@ember/-internals/metal/lib/observer.ts b/packages/@ember/-internals/metal/lib/observer.ts
@@ -95,7 +95,7 @@ export function activateObserver(target: object, eventName: string, sync = false
     activeObservers.get(eventName)!.count++;
   } else {
     let [path] = eventName.split(':');
-    let tag = combine(getChainTagsForKey(target, path));
+    let tag = combine(getChainTagsForKey(target, path, true));
 
     activeObservers.set(eventName, {
       count: 1,
@@ -159,14 +159,14 @@ export function resumeObserverDeactivation() {
 export function revalidateObservers(target: object) {
   if (ASYNC_OBSERVERS.has(target)) {
     ASYNC_OBSERVERS.get(target)!.forEach(observer => {
-      observer.tag = combine(getChainTagsForKey(target, observer.path));
+      observer.tag = combine(getChainTagsForKey(target, observer.path, true));
       observer.lastRevision = valueForTag(observer.tag);
     });
   }
 
   if (SYNC_OBSERVERS.has(target)) {
     SYNC_OBSERVERS.get(target)!.forEach(observer => {
-      observer.tag = combine(getChainTagsForKey(target, observer.path));
+      observer.tag = combine(getChainTagsForKey(target, observer.path, true));
       observer.lastRevision = valueForTag(observer.tag);
     });
   }
@@ -196,7 +196,7 @@ export function flushAsyncObservers(shouldSchedule = true) {
           try {
             sendEvent(target, eventName, [target, observer.path], undefined, meta);
           } finally {
-            observer.tag = combine(getChainTagsForKey(target, observer.path));
+            observer.tag = combine(getChainTagsForKey(target, observer.path, true));
             observer.lastRevision = valueForTag(observer.tag);
           }
         };
@@ -230,7 +230,7 @@ export function flushSyncObservers() {
           observer.suspended = true;
           sendEvent(target, eventName, [target, observer.path], undefined, meta);
         } finally {
-          observer.tag = combine(getChainTagsForKey(target, observer.path));
+          observer.tag = combine(getChainTagsForKey(target, observer.path, true));
           observer.lastRevision = valueForTag(observer.tag);
           observer.suspended = false;
         }

diff --git a/packages/@ember/-internals/metal/lib/tags.ts b/packages/@ember/-internals/metal/lib/tags.ts
@@ -54,19 +54,25 @@ export const CUSTOM_TAG_FOR = symbol('CUSTOM_TAG_FOR');
 // This is exported for `@tracked`, but should otherwise be avoided. Use `tagForObject`.
 export const SELF_TAG: string = symbol('SELF_TAG');
 
-export function tagForProperty(obj: unknown, propertyKey: string | symbol): Tag {
+export function tagForProperty(
+  obj: unknown,
+  propertyKey: string | symbol,
+  addMandatorySetter = false
+): Tag {
   if (!isObject(obj)) {
     return CONSTANT_TAG;
   }
 
   if (typeof obj[CUSTOM_TAG_FOR] === 'function') {
-    return obj[CUSTOM_TAG_FOR](propertyKey);
+    return obj[CUSTOM_TAG_FOR](propertyKey, addMandatorySetter);
   }
 
   let tag = tagFor(obj, propertyKey);
 
   if (DEBUG) {
-    setupMandatorySetter!(tag, obj, propertyKey);
+    if (addMandatorySetter) {
+      setupMandatorySetter!(tag, obj, propertyKey);
+    }
 
     // TODO: Replace this with something more first class for tracking tags in DEBUG
     (tag as any)._propertyKey = propertyKey;

diff --git a/packages/@ember/-internals/runtime/lib/mixins/-proxy.js b/packages/@ember/-internals/runtime/lib/mixins/-proxy.js
@@ -58,7 +58,7 @@ export default Mixin.create({
     return Boolean(get(this, 'content'));
   }),
 
-  [CUSTOM_TAG_FOR](key) {
+  [CUSTOM_TAG_FOR](key, addMandatorySetter) {
     let tag = tagFor(this, key);
 
     if (DEBUG) {
@@ -67,13 +67,13 @@ export default Mixin.create({
     }
 
     if (key in this) {
-      if (DEBUG) {
+      if (DEBUG && addMandatorySetter) {
         setupMandatorySetter(tag, this, key);
       }
 
       return tag;
     } else {
-      return combine([tag, ...getChainTagsForKey(this, `content.${key}`)]);
+      return combine([tag, ...getChainTagsForKey(this, `content.${key}`, addMandatorySetter)]);
     }
   },
 

diff --git a/packages/@ember/-internals/runtime/lib/system/array_proxy.js b/packages/@ember/-internals/runtime/lib/system/array_proxy.js
@@ -114,12 +114,12 @@ export default class ArrayProxy extends EmberObject {
     this._revalidate();
   }
 
-  [CUSTOM_TAG_FOR](key) {
+  [CUSTOM_TAG_FOR](key, addMandatorySetter) {
     if (key === '[]' || key === 'length') {
       // revalidate eagerly if we're being tracked, since we no longer will
       // be able to later on due to backtracking re-render assertion
       this._revalidate();
-      return combine(getChainTagsForKey(this, `arrangedContent.${key}`));
+      return combine(getChainTagsForKey(this, `arrangedContent.${key}`, addMandatorySetter));
     }
 
     return tagFor(this, key);