-
Notifications
You must be signed in to change notification settings - Fork 19
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
Proposal: full support for JSON Patch spec #6
Comments
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? |
hi @neftaly ,
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 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. |
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
} |
I'm also thinking about this option interface and providing another one here.
|
This would work. I suggest defaulting to 100% spec-compatible patches. |
I'm afraid not, |
Ok, fair enough. My reasoning is that it would be drop-in compatible with Immer (including V10+) for people who wish to migrate. |
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. |
hi @neftaly , The {
/**
* 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;
} |
Excellent, thank you! |
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.length
, which is not consistent with JSON Patch 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 ofnever
, and removeenablePatches
.usePatches
isalways
, the patches it produces will not exactly match JSON patch spec, but it will maintain the good performance that most uses of patches require.usePatches
isjson-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.The text was updated successfully, but these errors were encountered: