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

objectAt in hasMany relation after record removal still returns record #2968

Closed
denisovlev opened this issue Apr 1, 2015 · 7 comments
Closed

Comments

@denisovlev
Copy link

objectAt: function(index) {
  if (this.currentState[index]) {
    return this.currentState[index];
  } else {
    return this.canonicalState[index];
  }
},

This code in ManyArray returns record even after it was deleted from relation. Documentation for array says following:

If the given index is negative or is greater or equal than the array length, returns undefined.

Does current behavior means that we shouldn't use objectAt at hasMany relations?

@ghedamat
Copy link
Contributor

ghedamat commented Apr 2, 2015

👍 pretty sure I've just hit the same bug
will try to reproduce on a jsBin asap

@ghedamat
Copy link
Contributor

ghedamat commented Apr 3, 2015

Ok

I think I've isolated a possible way of reproduce

click on the jsbins and try to hit the remove button, the list should shorten.

if one simply uses the hasMany relationship it works
http://emberjs.jsbin.com/lejepa/5/edit?html,js,output

but if .objectsAt is appended then it's broken and removed elements are still returned:
http://emberjs.jsbin.com/lejepa/4/edit?html,js,output

as @denisovlev was mentioning I'd love to know whether this is actually a supported use case for the DS.ManyArray

thanks!

EDIT: pretty sure jsbins are useless now that I've actually read what @denisovlev wrote :P sry :D

@trianglegrrl
Copy link

👍 This seems to be the cause of one of the long-standing bugs in one of our apps. We're on beta 15.

@igorT
Copy link
Member

igorT commented May 8, 2015

I screwed up and thought this was the same bug as a more complex relationship bug I got stuck tackling. Apologies for that, I should've realized that this code was wrong.

This was a premature optimization because we didn't want to copy over canonical array to the current array every time, but rather access it lazily. However that proved hard, so for the initial implementation I ended up copying. However this was not removed and snuck through code review. :(
Is fixed by #3054

@igorT
Copy link
Member

igorT commented May 8, 2015

@trianglegrrl @lukegalea told me you had no bugs, feel free to ping me if things come up

@igorT igorT closed this as completed May 8, 2015
@ghedamat
Copy link
Contributor

ghedamat commented May 8, 2015

@igorT thanks!

@trianglegrrl
Copy link

@igorT thanks for looking at it!

Luke probably just forgot about this bug. :) I'll test it this weekend.

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

4 participants