Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX lts] Ensure setter CP's with dependent keys on curly components can be two way bound #19028

Merged
merged 7 commits into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -675,6 +676,25 @@ export class ComputedProperty extends ComputedDescriptor {
let ret;

try {
if (obj instanceof Ember.Component) {
richgt marked this conversation as resolved.
Show resolved Hide resolved
// 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#18417 / GH#19028 for details.
let descriptor: ComputedDescriptor;
descriptor = descriptorForProperty(obj, keyName);
let meta = metaFor(obj);
richgt marked this conversation as resolved.
Show resolved Hide resolved
if (
meta.isInitializing() &&
descriptor !== undefined &&
descriptor._dependentKeys !== undefined &&
descriptor._dependentKeys.length > 0
) {
richgt marked this conversation as resolved.
Show resolved Hide resolved
addObserver(obj, keyName, obj[PROPERTY_DID_CHANGE].bind(obj, keyName), undefined, true);
richgt marked this conversation as resolved.
Show resolved Hide resolved
}
}

beginPropertyChanges();

ret = this._set(obj, keyName, value);
Expand Down