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

Fix expando set #84

Merged
merged 7 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 8 additions & 12 deletions src/can-observable-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ObservableArray extends MixedInArray {
for(let i = 0, len = items && items.length; i < len; i++) {
this[i] = convertItem(new.target, items[i]);
}
const obj = this;
const instance = this;
// Define class fields observables
//and return the proxy
return new Proxy(this, {
Expand All @@ -56,21 +56,17 @@ class ObservableArray extends MixedInArray {
throw new Error('ObservableArray does not support a class field named items. Try using a different name or using static items');
}

const props = target.constructor.props;
let value = descriptor.value;

if (!value && typeof descriptor.get === 'function') {
value = descriptor.get();
// Don't overwrite static props
// that share the same name with a class field
const props = target.constructor.props;
if (props && props[prop]) {
instance[prop] = value;
return true;
}

if (value) {
if (props && props[prop]) {
obj[prop] = value;
return true;
}
// Make the property observable
return mixins.expando(target, prop, value);
}
return mixins.expando(target, prop, value);
}
});
}
Expand Down
49 changes: 32 additions & 17 deletions test/class-field-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,43 @@ QUnit.test('Throws on class field property named items', function (assert) {

});

QUnit.test('handle descriptor getter', function(assert) {
assert.expect(4);

const foo = new ObservableArray();

let _bar = "Hello";
Object.defineProperty(foo, "bar", {
get() {
return _bar;
},
set(val) {
_bar = val;
QUnit.test('set should work', function(assert) {
class Foo extends ObservableArray{}
const foo = new Foo();
foo.set("count", 3);
assert.equal(foo.count, 3);
});

QUnit.test('Class fields should not overwrite static props', function (assert) {
const done = assert.async();
assert.expect(5);

class MyList extends ObservableArray{
/* jshint ignore:start */
greetings = 'Hello';
/* jshint ignore:end */
static get props() {
return {
greetings: 'Bonjour'
};
}
});
assert.equal(foo.bar, 'Hello');
}

const aList = new MyList();

foo.on('bar', function (ev, newVal, oldVal) {
assert.equal(aList.greetings, 'Hello', 'Default valus is correct');
aList.on('greetings', function (ev, newVal, oldVal) {
assert.equal(oldVal, 'Hello', 'Old value is correct');
assert.equal(newVal, 'Hola', 'Value is updated');
assert.ok(ev, 'The class field is observable');
done();
});

foo.bar = 'Hola';
aList.greetings = 'Hola';

try {
aList.greetings = {foo: 'bar'};
} catch (error) {
assert.ok(error, 'Error thrown on the wrong type');
}
});