Merge pull request #19028 from richgt/richgt/18147-computed-property-…

…issue [BUGFIX lts] Ensure setter CP's with dependent keys on curly components can be two way bound
emberjs · Aug 5, 2020 · 3ef8495 · 3ef8495
2 parents 49383bd + dbebd35
commit 3ef8495
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 0 deletions.
diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts
@@ -38,6 +38,7 @@ import {
   referenceForKey,
   SimpleClassNameBindingReference,
 } from '../utils/bindings';
+
 import ComponentStateBucket, { Component } from '../utils/curly-component-state-bucket';
 import { processComponentArgs } from '../utils/process-args';
 import AbstractManager from './abstract';

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
@@ -2904,6 +2904,84 @@ moduleFor(
       this.assertText('initial value');
     }
 
+    ['@test GH#18417 - a two way binding flows upstream to a parent component through a CP']() {
+      let parent, child;
+      let ParentComponent = Component.extend({
+        init() {
+          this._super(...arguments);
+          parent = this;
+        },
+        string: 'Hello|World',
+      });
+
+      this.registerComponent('parent', {
+        ComponentClass: ParentComponent,
+        template: `{{child value=string}}
+
+        Parent String=<span data-test-parent-value>{{string}}</span>`,
+      });
+
+      let ChildComponent = Component.extend({
+        init() {
+          this._super(...arguments);
+          child = this;
+        },
+
+        a: null, // computed based on passed in value of `string`
+        b: null, // computed based on passed in value of `string`
+
+        value: computed('a', 'b', {
+          get() {
+            return this.a + '|' + this.b;
+          },
+
+          set(key, value) {
+            let vals = value.split('|');
+            set(this, 'a', vals[0]);
+            set(this, 'b', vals[1]);
+            return value;
+          },
+        }),
+      });
+
+      this.registerComponent('child', {
+        ComponentClass: ChildComponent,
+        template: '{{value}}',
+      });
+
+      this.render('{{parent}}');
+
+      this.assert.equal(parent.string, 'Hello|World', 'precond - parent value');
+      this.assert.equal(
+        this.element.querySelector('[data-test-parent-value]').textContent.trim(),
+        'Hello|World',
+        'precond - parent rendered value'
+      );
+
+      runTask(() => {
+        child.set('a', 'Foo');
+      });
+
+      this.assert.equal(parent.string, 'Foo|World', 'parent value updated');
+
+      this.assert.equal(
+        this.element.querySelector('[data-test-parent-value]').textContent.trim(),
+        'Foo|World',
+        'parent updated value rendered'
+      );
+
+      runTask(() => {
+        child.set('a', 'Hello');
+      });
+
+      this.assert.equal(parent.string, 'Hello|World', 'parent value reset');
+      this.assert.equal(
+        this.element.querySelector('[data-test-parent-value]').textContent.trim(),
+        'Hello|World',
+        'parent template reset'
+      );
+    }
+
     ['@test services can be injected into components']() {
       let service;
       this.registerService(

diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts
@@ -1,4 +1,5 @@
 import { Meta, meta as metaFor } from '@ember/-internals/meta';
+import { addObserver, PROPERTY_DID_CHANGE } from '@ember/-internals/metal';
 import { inspect, isEmberArray, toString } from '@ember/-internals/utils';
 import { assert, deprecate, warn } from '@ember/debug';
 import EmberError from '@ember/error';
@@ -672,6 +673,31 @@ export class ComputedProperty extends ComputedDescriptor {
       return this.volatileSet(obj, keyName, value);
     }
 
+    // ensure two way binding works when the component has defined a computed
+    // property with both a setter and dependent keys, in that scenario without
+    // the sync observer added below the caller's value will never be updated
+    //
+    // See GH#18147 / GH#19028 for details.
+    if (
+      this._dependentKeys !== undefined &&
+      this._dependentKeys.length > 0 &&
+      // These two properties are set on Ember.Component
+      typeof obj[PROPERTY_DID_CHANGE] === 'function' &&
+      (obj as any).isComponent &&
+      // ensure that we only run this once, while the component is being instantiated
+      metaFor(obj).isInitializing()
+    ) {
+      addObserver(
+        obj,
+        keyName,
+        () => {
+          obj[PROPERTY_DID_CHANGE](keyName);
+        },
+        undefined,
+        true
+      );
+    }
+
     let ret;
 
     try {