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

Calling chidlList.add to add an element that's already in the childList throws "out of bounds" error #509

Closed
rodrigojmr opened this issue Aug 3, 2023 · 0 comments · Fixed by #510 or #514
Labels
bug fix-in-review This issue has a PR in review

Comments

@rodrigojmr
Copy link

rodrigojmr commented Aug 3, 2023

We're now gradually bumping Lightning to the latest version and came across this when bumping to 2.5.0, and it should still occur in latest.

Caused by #314

This makes it so we have have to check if the item exists in the childList before calling add every time, at least for certain uses.

Say we have an item that may be attached and detached from different childLists, as it's something like a reusable visual focus element. When we've added that item to the childList before and try to add it again, the check using currentIndex and index (in this context, the length of the array) will be incorrect. Say currentIndex is 1 and the length of the childList is 2, so that's the index.

    addAt(item, index) {
        if (index >= 0 && index <= this._items.length) {
            let currentIndex = this._items.indexOf(item);
            // Never true when being called by `add`
            if (currentIndex === index) {
                return item;
            }
          ......
            if (currentIndex != -1) {
                this.setAt(item, index);
          ......
}

    setAt(item, index) {
        // Never true, throws out of bounds error
        if (index >= 0 && index < this._items.length) {

This worked in 2.4.0, even if it triggered this whole flow:

            let currentIndex = this._items.indexOf(item);
            if (currentIndex != -1) {
                if (currentIndex !== index) {
                    const fromIndex = currentIndex;
                    if (fromIndex !== index) {
                        this._items.splice(fromIndex, 1);
                        this._items.splice(index, 0, item);
                        this.onMove(item, fromIndex, index);
                    }
                }

Surely this could be optimized? Or is this the responsability of the application developers, and if so, shouldn't this be documented?

@rodrigojmr rodrigojmr changed the title Calling chidlList.add to add an element that's already in the childList throws an error. Calling chidlList.add to add an element that's already in the childList throws "out of bounds" error Aug 3, 2023
@uguraslan uguraslan linked a pull request Aug 31, 2023 that will close this issue
@uguraslan uguraslan added fix-in-review This issue has a PR in review bug labels Aug 31, 2023
@uguraslan uguraslan mentioned this issue Oct 11, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix-in-review This issue has a PR in review
Projects
None yet
2 participants