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

Array patching, over empty elements.. #287

Open
KpjComp opened this issue Nov 2, 2021 · 2 comments
Open

Array patching, over empty elements.. #287

KpjComp opened this issue Nov 2, 2021 · 2 comments

Comments

@KpjComp
Copy link

KpjComp commented Nov 2, 2021

has a slight problem with empty array elements and applying diffs.

Lets assume we have we have an array [1,,3,,5], and then create a patch for [1,,3,,5,,7,,9], The patch created is correct, eg: add for array element 6 & 8 with values 7 & 9. But if we apply this patch to the original, you will get [1,,3,,5,7,9].. IOW: it will scrunch the newly added array elements and put the first array element at index 5 & 6, instead of 6 & 8.

This function ->

var arrOps = {
    add: function (arr, i, document) {
        if (helpers_js_1.isInteger(i)) {
            arr.splice(i, 0, this.value);
        }
        else { // array props
            arr[i] = this.value;
        }
        // this may be needed when using '-' in an array
        return { newDocument: document, index: i };
    },

Now I'm not 100% sure why splice was used here, unless it's maybe a performance optimization, but the problem with splice here, if you did splice(6, 0, value), the index position it will use if the index is greater than length, is the length (wrong). IOW: It will update index 5, and not index 6, and then index 8 will be 6 (again because of length).

Now a simple solution is just don't use splice, and use arr[i] = , or alternatively set the length of the array first before doing splice.

@wickning1
Copy link

The spec says "The specified index MUST NOT be greater than the number of elements in the array."

I think the error here is on the creation of the patch. It should be adding undefined elements before the 7 and the 9.

@KpjComp
Copy link
Author

KpjComp commented Jul 12, 2022

@wickning1 After doing the modification I mention, I've not had any issues with patching arrays.

But what is odd is that this issue is still marked OPEN, and for such a simple bug, with a very simple fix, this worries me a tad. IOW: is this project dead?

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

2 participants