Ember.warn when ManyArray.objectAt() is undefined #4896
Conversation
|
Thanks @bagby. |
| @@ -136,7 +136,12 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { | |||
| objectAt(index) { | |||
| let object = this.currentState[index]; | |||
| //Ember observers such as 'firstObject', 'lastObject' might do out of bounds accesses | |||
| if (object === undefined) { return; } | |||
| if (object === undefined) { | |||
There was a problem hiding this comment.
this needs to bounds check that index is not length-1 or 0 because of the greediness of firstObject and lastObject.
There was a problem hiding this comment.
for future travelers the issue @runspired is referring to is emberjs/ember.js#14843
|
Thank you for the contribution, unfortunately @hjdivad (who I am pairing with right now) forgot that #4758 itself is no longer valid. So again, thank you for the contribution, and we are sorry for misleading you :( :( :( The PR as is, is slightly in-correct as warning for out of bounds wasn't the original intention, rather it was non out of bounds check, but the internalModel returned being unloaded is the case where the warning should have occurred. That being said @hjdivad and @igorT changed the semantics, so that the case #4758 was opened to to fix does not occur, as the entire manyArray (in this scenario) is now destroyed. |
This reverts commit 44ffbff. More details: #4896 (comment)
First pass at #4758 . Not sure that there's a commit prefix that is applicable here, it's more to help with debugging, and to provide some data points as to the prevalence of the problem.