Skip to content

Commit

Permalink
Throw an error from ko.isObservable if it detects an observable from …
Browse files Browse the repository at this point in the history
…another Knockout instance. fixes #1344

ko.isObservable(ko.computed) now returns false. fixes #1958
  • Loading branch information
mbest committed Dec 3, 2016
1 parent d2a9e44 commit cdd2741
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 21 deletions.
18 changes: 18 additions & 0 deletions spec/dependentObservableBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ describe('Dependent Observable', function() {
expect(ko.isObservable(instance)).toEqual(true);
});

it('Should not advertise that ko.computed is observable', function () {
expect(ko.isObservable(ko.computed)).toEqual(false);
});

it('Should advertise that instances are computed', function () {
var instance = ko.computed(function () { });
expect(ko.isComputed(instance)).toEqual(true);
Expand All @@ -26,6 +30,20 @@ describe('Dependent Observable', function() {
expect(ko.isWritableObservable(instance)).toEqual(false);
});

it('ko.isComputed should return false for non-computed values', function () {
ko.utils.arrayForEach([
undefined,
null,
"x",
{},
function() {},
ko.observable(),
(function() { var x = ko.computed(function() {}); x.__ko_proto__= {}; return x; }())
], function (value) {
expect(ko.isComputed(value)).toEqual(false);
});
});

it('Should require an evaluator function as constructor param', function () {
expect(function () { ko.computed(); }).toThrow();
});
Expand Down
28 changes: 28 additions & 0 deletions spec/observableBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,39 @@ describe('Observable', function() {
expect(ko.isObservable(instance)).toEqual(true);
});

it('Should not advertise that ko.observable is observable', function () {
expect(ko.isObservable(ko.observable)).toEqual(false);
});

it('Should advertise that instances are not computed', function () {
var instance = ko.observable();
expect(ko.isComputed(instance)).toEqual(false);
});

it('Should advertise that instances are not pure computed', function () {
var instance = ko.observable();
expect(ko.isPureComputed(instance)).toEqual(false);
});

it('ko.isObservable should return false for non-observable values', function () {
ko.utils.arrayForEach([
undefined,
null,
"x",
{},
function() {},
new ko.subscribable()
], function (value) {
expect(ko.isObservable(value)).toEqual(false);
});
});

it('ko.isObservable should throw exception for value that has fake observable pointer', function () {
var x = ko.observable();
x.__ko_proto__= {};
expect(function () { ko.isObservable(x); }).toThrow();
});

it('Should be able to write values to it', function () {
var instance = new ko.observable();
instance(123);
Expand Down
8 changes: 3 additions & 5 deletions src/subscribables/dependentObservable.js
Original file line number Diff line number Diff line change
Expand Up @@ -442,18 +442,16 @@ if (ko.utils.canSetPrototype) {
ko.utils.setPrototypeOf(computedFn, ko.subscribable['fn']);
}

// Set the proto chain values for ko.hasPrototype
// Set the proto values for ko.computed
var protoProp = ko.observable.protoProperty; // == "__ko_proto__"
ko.computed[protoProp] = ko.observable;
computedFn[protoProp] = ko.computed;

ko.isComputed = function (instance) {
return ko.hasPrototype(instance, ko.computed);
return (typeof instance == 'function' && instance[protoProp] === ko.computed);
};

ko.isPureComputed = function (instance) {
return ko.hasPrototype(instance, ko.computed)
&& instance[computedState] && instance[computedState].pure;
return ko.isComputed(instance) && instance[computedState] && instance[computedState].pure;
};

ko.exportSymbol('computed', ko.computed);
Expand Down
26 changes: 10 additions & 16 deletions src/subscribables/observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,19 @@ if (ko.utils.canSetPrototype) {
var protoProperty = ko.observable.protoProperty = '__ko_proto__';
observableFn[protoProperty] = ko.observable;

ko.hasPrototype = function(instance, prototype) {
if ((instance === null) || (instance === undefined) || (instance[protoProperty] === undefined)) return false;
if (instance[protoProperty] === prototype) return true;
return ko.hasPrototype(instance[protoProperty], prototype); // Walk the prototype chain
ko.isObservable = function (instance) {
var proto = typeof instance == 'function' && instance[protoProperty];
if (proto && proto !== ko.observable && proto !== ko.computed) {
throw Error("Invalid object that looks like an observable; possibly from another Knockout instance");
}
return !!proto;
};

ko.isObservable = function (instance) {
return ko.hasPrototype(instance, ko.observable);
}
ko.isWriteableObservable = function (instance) {
// Observable
if ((typeof instance == 'function') && instance[protoProperty] === ko.observable)
return true;
// Writeable dependent observable
if ((typeof instance == 'function') && (instance[protoProperty] === ko.dependentObservable) && (instance.hasWriteFunction))
return true;
// Anything else
return false;
}
return (typeof instance == 'function' && (
(instance[protoProperty] === ko.observable) || // Observable
(instance[protoProperty] === ko.computed && instance.hasWriteFunction))); // Writable computed observable
};

ko.exportSymbol('observable', ko.observable);
ko.exportSymbol('isObservable', ko.isObservable);
Expand Down

0 comments on commit cdd2741

Please sign in to comment.