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

[bug] Invalid patch produced from array.splice() call #68

Closed
davidye opened this issue Oct 28, 2024 · 4 comments
Closed

[bug] Invalid patch produced from array.splice() call #68

davidye opened this issue Oct 28, 2024 · 4 comments

Comments

@davidye
Copy link

davidye commented Oct 28, 2024

const x: string[] = ["a"];
const [_, patches] = create(
  x,
  (draft) => {
    draft.splice(0, 1);
  },
  { enablePatches: true }
);

console.log(patches);

This produces:

[{ op: "replace", path: "length"], value: 0 }]

When it should be a "remove" operation

@davidye
Copy link
Author

davidye commented Oct 28, 2024

I did see that this may be intentional from: #65

But I think the generated patch should not include "length" for it to match the JSON patch spec? FWIW immer does handle this case with "remove".

@unadlib
Copy link
Owner

unadlib commented Oct 28, 2024

hi @davidye , by default, Mutative’s patch generation rules differ slightly from the JSON Patch spec. The doc: https://github.com/unadlib/mutative?tab=readme-ov-file#createstate-fn-options.

You can set arrayLengthAssignment: false to meet your requirements.

const x: string[] = ['a'];
const [_, patches] = create(
  x,
  (draft) => {
    draft.splice(0, 1);
  },
  {
    enablePatches: {
      arrayLengthAssignment: false,
    },
  }
);

console.log(patches); // [ { op: 'remove', path: [ 0 ] } ]

Here #6 is more related discussion.

@unadlib
Copy link
Owner

unadlib commented Oct 28, 2024

This would work. I suggest defaulting to 100% spec-compatible patches.

I'm afraid not, arrayLengthAssignment should be true by default, otherwise most users using patches will suffer additional performance loss.

#6 (comment)

@davidye
Copy link
Author

davidye commented Oct 28, 2024

Oh I see, thank you! Excellent library by the way this is exactly what I was looking for.

@unadlib unadlib closed this as completed Oct 30, 2024
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