Skip to content

Commit

Permalink
Bug fix for overwritten object clean-up.
Browse files Browse the repository at this point in the history
* It's possible for a parent object to have multiple different
references to the same object. If an object gets overwritten, we want to
stop watching it but *only* if there are no other references to that
object under the parent object and all nested objects.
  • Loading branch information
ElliotNB committed Nov 11, 2018
1 parent e491901 commit 62f1844
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 12 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

https://github.com/elliotnb/observable-slim

Version 0.1.0
Version 0.1.1

Licensed under the MIT license:

Expand Down
33 changes: 29 additions & 4 deletions observable-slim.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Observable Slim
* Version 0.1.0
* Version 0.1.1
* https://github.com/elliotnb/observable-slim
*
* Licensed under the MIT license:
Expand Down Expand Up @@ -342,7 +342,7 @@ var ObservableSlim = (function() {
// orphaned and grow memory usage. we excute this on a setTimeout so that the clean-up process does not block
// the UI rendering -- there's no need to execute the clean up immediately
setTimeout(function() {

if (typeOfTargetProp === "object" && targetProp !== null) {

// check if the to-be-overwritten target property still exists on the target object
Expand All @@ -353,6 +353,32 @@ var ObservableSlim = (function() {
for (var i = 0, l = keys.length; i < l; i++) {
if (target[keys[i]] === targetProp) return;
}

var stillExists = false;

// now we perform the more expensive search recursively through the target object.
// if we find the targetProp (that was just overwritten) still exists somewhere else
// further down in the object, then we still need to observe the targetProp on this observable.
(function iterate(target) {
var keys = Object.keys(target);
for (var i = 0, l = keys.length; i < l; i++) {
var property = keys[i];
if (target[property].__isProxy === true) {
var nestedTarget = target[property].__getTarget;
} else {
var nestedTarget = target[property];
}
if (target[property] instanceof Object && target[property] !== null) iterate(nestedTarget);
if (target[property] === targetProp) {
stillExists = true;
return;
}
};
})(target);

// even though targetProp was overwritten, if it still exists somewhere else on the object,
// then we don't want to remove the observable for that object (targetProp)
if (stillExists === true) return;

// loop over each property and recursively invoke the `iterate` function for any
// objects nested on targetProp
Expand All @@ -365,7 +391,6 @@ var ObservableSlim = (function() {
}

// if there are any existing target objects (objects that we're already observing)...
//var c = targets.indexOf(obj);
var c = -1;
for (var i = 0, l = targets.length; i < l; i++) {
if (obj === targets[i]) {
Expand Down Expand Up @@ -660,4 +685,4 @@ var ObservableSlim = (function() {
})();

// Export in a try catch to prevent this from erroring out on older browsers
try { module.exports = ObservableSlim; } catch (err) {};
try { module.exports = ObservableSlim; } catch (err) {};
2 changes: 1 addition & 1 deletion observable-slim.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "observable-slim",
"description": "Observable Slim is a singleton that utilizes ES6 Proxies to observe changes made to an object and any nested children of that object. It is intended to assist with state management and one-way data binding.",
"version": "0.1.0",
"version": "0.1.1",
"main": "observable-slim.js",
"devDependencies": {
"babel-core": "6.26.0",
Expand Down
29 changes: 24 additions & 5 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -635,20 +635,39 @@ function suite(proxy) {
});


// This test currently does not have an expectation or assertion. For now it simply
// ensures that the garbage clean-up code runs and that the garbage clean-up doesn't throw an error.
// When you overwrite a property that points to an object, Observable-Slim will perform a clean-up
// process to stop watching objects that are no longer a child of the parent (top-most) observed object.
// However, if a reference to the overwritten object exists somewhere else on the parent observed object, then we
// still need to watch/observe that object for changes. This test verifies that even after the clean-up process (10 second delay)
// changes to an overwritten object are still monitored as long as there's another reference to the object.
it('31. Clean-up observers of overwritten (orphaned) objects.', (done) => {

var data = {"testing":{"test":{"testb":"hello world"},"testc":"hello again"},"blah":{"tree":"world"}};
var dupe = {"duplicate":"is duplicated"};
data.blah.dupe = dupe;
data.dupe = dupe;
var changeCnt = 0;

var p = ObservableSlim.create(data, false, function(changes) {
return null;
changeCnt++;
if (changeCnt === 1) {
expect(p.testing.test).to.be.an("undefined");
} else if (changeCnt === 2) {
expect(p.dupe).to.be.an("object");
expect(p.dupe.duplicate).to.be.an("undefined");
} else {
expect(p.blah.dupe.duplicate).to.equal("should catch this change");
done();
}
});

p.testing = {};
p.dupe = {};


setTimeout(function() {
done();
},10000);
p.blah.dupe.duplicate = "should catch this change";
},10500);

});

Expand Down

0 comments on commit 62f1844

Please sign in to comment.