-
Notifications
You must be signed in to change notification settings - Fork 493
feat(types): add JSONPatchFormatter #316
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
Conversation
narrow down Operation type
|
@benjamine what do you think? are we good? |
|
Any updates here? :) |
|
BUMP - any updates on this? |
|
@sghoweri we eventually created a fork with more typescript support: https://github.com/contentful/jsondiffpatch I also started an independent project to create patches: https://github.com/marcolink/generate-json-patch |
|
@marcolink oh my gosh you're a lifesaver (was just dealing with the Typescript typing issues with this package) 🙏🏼 |
Oh snap. This is precisely why we started digging into this library to begin with (array diffing combined with JSON patches). 👀 |
Methuselah96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay review. Don't feel you need to update this PR, we plan on converting the codebase to TypeScript soon anyway, so these changes will likely get overridden, but feel free to update the PR with the requested changes and we can get it merged.
src/index.d.ts
Outdated
| } | ||
|
|
||
| type AddOperation = { | ||
| opp: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like these should be op and not opp (on all the operations)?
|
|
||
| export type Operation = AddOperation | RemoveOperation | ReplaceOperation | MoveOperation | ||
|
|
||
| export interface JSONPatchFormatter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally JSONPatchFormatter would inherit from Formatter, but we can handle that in a future PR.
|
@Methuselah96 for now we found our way around - looking forward to see the migration happening :) |
Add type definition for JSON Patch formatter matching official
Operationdefinition from http://jsonpatch.com