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

Manually created observables doesn't work if initially created with model = null #161

Open
joakimkemeny opened this issue Jul 31, 2017 · 5 comments

Comments

@joakimkemeny
Copy link
Contributor

joakimkemeny commented Jul 31, 2017

This issue is a continuation of #82. In version 0.19.1 you changed the way createObservables work and also provided an excellent example on how to update my code for that fix, so huge thanks.

Unfortunately I didn't test this until now (I'm a consultant and have been away from this project for a while) and version 0.19.1 introduced a new problem if the model is initially set to null that prevents me from updating (and I really want to update for the memory fixes).

This example (almost the same as in #82) worked fine in 0.19.0:

var children = new Backbone.Collection([
    new Backbone.Model({name:"Charles"}),
    new Backbone.Model({name:"Eve"})
]);

var parent = new Backbone.Model({
    name:"Bob", children:children
});

var subFactory = function (model) {
    var subVm = new kb.ViewModel(model);
    subVm.cid = kb.ko.computed(function () {
        return model.cid;
    });
    return subVm;
};

var vm = new kb.ViewModel(null, {
    excludes : ["children"]
});

// Do other stuff here that has to be done
// before I add the factory.

vm.shareOptions().factory.
    addPathMapping("children.models", subFactory);
vm.createObservables(null, ["children"]);

vm.model(parent);

console.log(vm.name());
console.log(vm.children()[0].cid());

This is obviously a strange example since I have the model and could just pass it instead of using null but in the real application it's not as easy and this is the smallest I could make the example.

In 0.19.1 I had to change the last part as per you suggestions in #82 to look like this:

...

var vm = new kb.ViewModel(null, {
    excludes : ["children"],
    factories: {"children.models": subFactory}
});

vm.children = kb.observable(null, "children", vm.shareOptions());

...

This will fail but if I pass the model directly instead of passing null and later setting the model it works fine.

I've tried really hard to find some kind of workaround for this but I've failed hard so any help would be hugely appreciated.

Thanks.

@Amsvartner
Copy link

Amsvartner commented Oct 11, 2017

👍 I would also really like to see a solution to this problem. Anyone?

kmalakoff added a commit that referenced this issue Oct 23, 2017
@kmalakoff
Copy link
Owner

Sorry for the delay. I've taken a look and fixed the example:

Was:

// Passing in parent instead of null works but in my case I don't have parent
// here so I must be able to set it later using vm.model(parent).
vm.children = kb.observable(null, "children", vm.shareOptions());

vm.model(parent);

ko.applyBindings(vm, document.getElementById("some-div"));

console.log(vm.name());
console.log(vm.children()[0].cid());

Fixed to:

// Passing in parent instead of null works but in my case I don't have parent
// here so I must be able to set it later using vm.model(parent).
vm.children = kb.collectionObservable(null, "children", vm.shareOptions());

vm.model(parent);

ko.applyBindings(vm, document.getElementById("some-div"));

console.log(vm.name());
console.log(vm.children().length);

The first problem was using a kb.observable which will default to null because that it an indication there is no value. If you want an array, you should use a kb.collectionObservable.

The next problem was trying to reference a cid() of the first value in an empty array. I changed it to length instead so it could be guaranteed to be a valid express.

Let me know if this solves it for you!

@joakimkemeny
Copy link
Contributor Author

Thank you so much for your efforts but this doesn't really fix it though. If you check the issue-0.19.0 example you'll see that the two children actually show up so the [0].cid() is not a problem. In your solution the array has a length of zero and it should be 2.

@kmalakoff
Copy link
Owner

Sorry, I didn't fully appreciate the differences in the code.

I was able to get it working by adding another line:

Was:

vm.children = kb.observable(null, "children", vm.shareOptions());
vm.model(parent);

Added:

vm.children = kb.observable(null, "children", vm.shareOptions());
vm.model(parent);
vm.children.model(parent);

Basically, the difference between a kb.observable created as part of the view model and independently with null is that the view model does not know how to track the model, eg. it was created independently from the view model.

Would setting the model on the manual ko.observable work for you? If not, I could explore options to set the model on each observable when the model changes.

@kmalakoff
Copy link
Owner

I've been thinking about if there is an easy way to automate this. The problem I have is that if you do not create the observable through the view model, it doesn't know that it should update the model in the manually created observable.

If you are only changing the model once, the pattern I showed should work. If you need to change it multiple times, you could do something like to make the synchronization automatic:

vm.children = kb.observable(null, "children", vm.shareOptions());
ko.computed(() => { vm.children.model(vm.model()); })

vm.model(parent);

Otherwise, I'm a little unsure how to support this use case so that there are not side effects by assuming all models on a view model's observables should be identical.

If the above hack doesn't work for you as a solution, maybe we need to go back to the original case that you are trying to solve and figure out if there is a better way to solve it that preserves ownership of the observables by the view model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants