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

Splice method always return last index of array instead of deleted index. #28

Closed
joharzmn opened this issue Mar 4, 2019 · 2 comments
Closed

Comments

@joharzmn
Copy link

joharzmn commented Mar 4, 2019

I have this array:

todos: [
          {
            id: "a81823a9-2afa-4427-963f-01693809fe0e", //  Should be called uniqueId
            name: "Task1",
            done: true
          },
          {
            id: "1daa7f75-9de9-4925-a94e-0169380a6efa",
            name: "Task2",
            done: false
          }
        ]

proxy.splice(0, 1);

Changes I got:

 [ { type: 'update',
    target: [ [Object], [Object], __length: 2 ],
    property: '0',
    newValue: 
     { id: '1daa7f75-9de9-4925-a94e-0169380a6efa',
       name: 'Task2',
       done: false },
    previousValue: 
     { id: 'a81823a9-2afa-4427-963f-01693809fe0e',
       name: 'Task1',
       done: true },
    currentPath: '0',
    jsonPointer: '/0',
    proxy: [ [Object], [Object], __length: 2 ] } ] 
 [ { type: 'delete',
    target: [ [Object], , __length: 2 ],
    property: '1',
    newValue: null,
    previousValue: 
     { id: '1daa7f75-9de9-4925-a94e-0169380a6efa',
       name: 'Task2',
       done: false },
    currentPath: '1',
    jsonPointer: '/1',
    proxy: [ [Object], , __length: 2 ] } ] 
 [ { type: 'update',
    target: [ [Object], __length: 1 ],
    property: 'length',
    newValue: 1,
    previousValue: 2,
    currentPath: 'length',
    jsonPointer: '/length',
    proxy: [ [Object], __length: 1 ] } ]

In the change where change type is 'delete', It should return 0 index but it is always returning last index of array.

@ElliotNB
Copy link
Owner

ElliotNB commented Mar 4, 2019

@joharzmn Unfortunately that's just how Proxy reports the splice mutations -- very similar to issue #16. This is how the browser processes arr.splice(0,1); where arr = ["a","b"];

  1. Copy arr[1] to arr[0].
  2. Delete arr[1].
  3. Set arr.length to 1.

In issue #16, I listed out two possible solutions to this issue:

As far as I know, there we two ways you could accomplish the desired behavior. You could either:

  1. Wrap the Array mutation methods so you can capture the push, splice, etc invocation before the value is mutated.
  2. Permanently enable domDelay so that changes are reported in batches and if the set of array mutations matches the pattern for a push, splice or anything else, then emit a custom event for that array mutation rather than the batch of raw mutations.

The first option isn't really viable because wrapping Array mutation methods is just asking for a conflict with other libraries and the ES5 Proxy polyfill already wraps the array mutation methods (i.e., things would get complicated and slow if we double wrapped the Array mutation methods).

The second option seems possible, but it will have it's own challenges. I'm not certain if all browsers implementation of Proxy reports change events with a push, splice, etc in the same order. The second option would also prevent us from fulfilling another enhancement request where we allow users to intercept changes. I'm thinking it might be best if the second option were enabled as a special mode that's turned off by default. Either way, it'll take quite a bit of time and effort to put together that functionality and verify it's working across all major browsers and the polyfill.

Originally posted by @ElliotNB in #16 (comment)

The crux of the issue is that Proxy describes .splice mutations in a verbose manner. Unfortunately, translating what Proxy reports for into something more concise will be difficult and could have cross-browser irregularities.

I've left issue #16 open as a reminder to investigate/pursue option two (listed above).

@joharzmn
Copy link
Author

joharzmn commented Mar 5, 2019

Okay Thank you @ElliotNB

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

2 participants