Fix/es5 array delete error

[luruozhou]

when delete array items in using es5 mode, the produce doesn't work.

for example:

   let base = [1,2,3];
   let changes = []
   let inverseChanges = []

   let fork = produce(
       base,
       draft => {
           delete draft[1];
       },
       (patches, inversePatches) => {
           console.log(patches,inversePatches)
           changes.push(...patches)
           inverseChanges.push(...inversePatches)
       }
   )
   console.log(fork, fork === base, changes, inverseChanges) // [1,2,3]  true [] []

finally,we get that fork is the same reference with base, the same time we get that the length of patches and inversePatches is zero.

[coveralls]

Coverage increased (+0.9%) to 94.089% when pulling e52f02f on luruozhou:fix/es5-array-delete-error into 211f5cf on mweststrate:master.

[mweststrate]

Why is markChanged called here again?

[luruozhou]

yeah... sorry, it's a mistake.

[mweststrate]

I'm fine by just explicitly forbidding it (I mean, it is pretty weird to call delete on an array, as splice is the idiomatic way to remove entries in the middle). That als simplifies the rest of the implementation

[luruozhou]

In both proxy and es5 mode, when delete an array item wont't cause any error, unless calling applyPatches with the patches. I think it's better to use the patches to replay the actions

[luruozhou]

for example

   let anArray = [1,2,3];
   let changes = [];
   produce(anArray, d => {
       delete d[1];
   }, (p, i) => {
       changes = [...p];
   })
  
  let arr1 = applyPatches([1, 2], changes)
  console.log(arr1)  // [1]

  let arr2 = applyPatches([1, 2, 3, 4], changes)
  console.log(arr2) //  throw "Remove can only remove the last key of an array"

the same patches apply on some data will lead to different result. It's so confused for me.

[mweststrate]

I would simplify the entire solution to have the proxy trap throw on delete if the target is an array. delete should not be used on arrays in producers period. It will keep the implementation of immer faster as well, compared to the current changes in the PR. If it was super easy it would be fine to support, but there is no real value in supporting it imho, so no reason to jump through hoops to get it working.

[luruozhou]

Ok, that's may be a good idea.

[mweststrate]

Thanks for catching this error! I think it should be fine to just throw an error when delete is used on array, as I think it is an extremely rare case and splice should have been used instead

[luruozhou]

I hope that the code can get the same behavior in both proxy and es5 mode, this is why I open this pr.

[luruozhou]

I also find other two questions in the process. In this PR, I think I've resolved them.
See the code sample below.
1、

   let base = [1,2, {name:"jim"}];
   let changes = []

   let fork = produce(
       base,
       draft => {
           delete draft[2].name;
       },
       (patches, inversePatches) => {
           changes.push(...patches)
       }
   )
   console.log(changes) //[]   deleting props from object should save the patches

2、

   let base =[1,2,3];

   let fork = produce(
       base,
       draft => {
           draft.name = "anArray"
       }
   )
   console.log(fork === base) //true   can add some non-index key on arrays?

I'm not sure if I should open an issue or another PR , but I wish to get some suggestions from @mweststrate , expect for your replies.