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

foreach doubles last entries under certain circumstances #2594

Open
dbogatz opened this issue May 28, 2022 · 4 comments
Open

foreach doubles last entries under certain circumstances #2594

dbogatz opened this issue May 28, 2022 · 4 comments
Milestone

Comments

@dbogatz
Copy link

dbogatz commented May 28, 2022

See: https://codepen.io/coyer/pen/zYRRqyo?editors=1010

HTML:

<script src="https://cdnjs.cloudflare.com/ajax/libs/knockout/3.5.0/knockout-min.js"></script>
<div data-bind="foreach: $root.list[$root.key()]">
  <div data-bind="text: $data"></div>
</div>

Script:

function AppViewModel() {
  this.key = ko.observable();
  this.list = {};
}
 
var vm = new AppViewModel();
ko.options.deferUpdates = true;
ko.applyBindings(vm);
setTimeout(()=>vm.list["test"] = ko.observableArray(["apple", "orange"]),100);
setTimeout(()=>vm.key("test"), 200);
setTimeout(()=>{
  vm.key.valueHasMutated();
  vm.list['test'](["cat", "dog", "mice", "bird"]);
}, 300);

Expected Output after 300ms:
"cat", "dog", "mice", "bird"
but is
"cat", "dog", "mice", "bird", "mice", "bird"

But works with "ko.options.deferUpdates = false;"
or without "vm.key.valueHasMutated();" before setting new list
or new list has same length as old list.

Anyway - thanks for the good job on knockout!

@dbogatz
Copy link
Author

dbogatz commented May 28, 2022

Also somewhat weird:

If I have two foreach on my page only the first one will be affected.

@miellaby
Copy link
Contributor

miellaby commented Jun 2, 2022

I've got a table component which duplicates lines for no reason. My component life cycle looks like your example. I'm happy you was able to pinpoint the issue in a simple example.

By the way, your demo link is broken in the issue description.

@dbogatz
Copy link
Author

dbogatz commented Jun 3, 2022

Thx, I fixed the link in my description and I try to avoid valueHasMutated for now on.
In my project it was used to trigger some computed's who access other changed non-observable data; it seems to put them in an observable too would be a good choice anyway.

@mbest
Copy link
Member

mbest commented Mar 7, 2023

I think this will be fixed with c41c1c3, but I'll double check.

@mbest mbest added this to the 3.5.2 milestone Mar 7, 2023
@mbest mbest added the type: bug label Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants