Proposal: full support for JSON Patch spec

[unadlib]

Mutative v0.3.2 does not fully support the JSON Patch spec.

• path type is an array, not a string as defined in the JSON patch spec.
• The patches generated by Mutative array clearing are a modification of the array length, which is not consistent with JSON Patch spec.
• Need to support JSON Pointer spec.

Since standard JSON patches are often used to sync backend with front-end state, compliance with JSON patch standard is necessary. However, considering array clearing patches will bring predictable performance loss, and has path type conversion issues.

We propose to add the option usePatches: 'json-patch' | 'never' | 'always', with the default value of never, and remove enablePatches.

• If the option usePatches is always, the patches it produces will not exactly match JSON patch spec, but it will maintain the good performance that most uses of patches require.
• If the option usePatches is json-patch, it produces patches that will be fully compliant with JSON patch spec, which has a slight performance penalty, and it ensures that the data it produces can be passed to other backend APIs based on JSON patch spec.

[neftaly]

Hi there. Any chance you could please post an example of a patch for mutative array clearing (or a link to a test)? Is this behavior different to immer patches?

[unadlib]

hi @neftaly ,

fast-json-patch is a JavaScript implementation that conforms to JSON Patch spec.

import { compare } from 'fast-json-patch';

const stateA = { list: [1, 2] };
const stateB = { list: [] };
const patches = compare(stateA, stateB);

expect(patches).toEqual([
  { op: 'remove', path: '/list/1' },
  { op: 'remove', path: '/list/0' },
]);

This is the Patches example in Immer v9.0.18.

import { produceWithPatches, enableAllPlugins } from 'immer';

enableAllPlugins();

const stateA = { list: [1, 2] };

const [, patches, inversePatches] = produceWithPatches(
  stateA,
  (draft) => {
    draft.list.length = 0;
  }
);

expect(patches).toEqual([
  { op: 'replace', path: ['list', 'length'], value: 0 },
]);

This is the Patches example in Mutative v0.3.2.

import { create } from 'mutative';

const stateA = { list: [1, 2] };

const [, patches, inversePatches] = create(
  stateA,
  (draft) => {
    draft.list.length = 0;
  },
  {
    enablePatches: true,
  }
);

expect(patches).toEqual([
  { op: 'replace', path: ['list', 'length'], value: 0 },
]);

We see that it is consistent with the Immer v9.0.18 implementation, but neither is compliant with the JSON Patch spec. One of Immer v10 plans on Don't use 'array.length' assignment in JSON patches. It will incur a predictable loss of performance.

After trading off performance and other issues, we think it should be a configurable option. This will help more users who need high-performance patches feature.

[neftaly]

Thank you for explaining this. For my use case (an immer wrapper for a third-party mutable object) I would like paths as arrays, but array patches to follow spec until I can code my own optimization. Is it possible to turn on individual features like the following?

{
  enablePatches: true,
  pathsAsArrays: true,
  arrayLengthChanges: false
}

[unadlib]

I'm also thinking about this option interface and providing another one here.

{
  enablePatches: boolean | { pathAsArray: boolean; arrayLengthAssignment: boolean }
}

[neftaly]

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

[unadlib]

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.

[neftaly]

Ok, fair enough. My reasoning is that it would be drop-in compatible with Immer (including V10+) for people who wish to migrate.
Edit: Except for paths, whoops

[unadlib]

I can understand, but upgrading from Immer v9 to Immer v10 itself has also compatibility issues, it's a breaking change. I mean Mutative just needs to be clearly described in the migration documentation.

I may implement this proposal in the last few days.

[unadlib]

hi @neftaly ,

The Patch options interface is as follows, I have released the latest version v0.4.0.

{
  /**
   * The default value is `true`. If it's `true`, the path will be an array, otherwise it is a string.
   */
  pathAsArray?: boolean;
  /**
   * The default value is `true`. If it's `true`, the array length will be included in the patches, otherwise no include array length.
   */
  arrayLengthAssignment?: boolean;
}

[neftaly]

Excellent, thank you!