feat(json): add reviver support to GET, MGET, and ARRPOP

[JeanSamGirard]

Allows passing an optional JSON.parse reviver function to transformReply. This enables automatic rehydration of types (like Dates) that are not natively supported by JSON.

Modified:

• JSON.GET: Added reviver to GetOptions
• JSON.MGET: Added reviver to arguments
• JSON.ARRPOP: Added reviver to ArrPopOptions
• generic-transformers: Added reviver to transformRedisJsonReply and transformRedisJsonNullReply arguments

Utilized .preserve to pass reviver through commands.

Description

I need to store arbitrary data structures in redis that may contain date objects or class instances that need to be rehydrated after being stringified, I believe others may have this need.

Commands that return JSON already needed to go through JSON.parse so this looked like a logical and easy feature to implement with the existing code structure. This is why I went ahead with making a pull request to add this feature myself instead of just creating an issue. (No linked issues)

PS: This is my first time contributing so if I'm doing anything wrong please tell me, thank you.

Checklist

• Does npm test pass with this change (including linting)?
• Is the new or changed code fully tested?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

---

Note

Medium Risk
Changes how JSON replies are parsed for JSON.GET, JSON.MGET, and JSON.ARRPOP by optionally running a caller-provided JSON.parse reviver, which could affect runtime types/behavior in downstream code. Scope is limited to JSON command reply transformation and covered by new tests.

Overview
Adds an optional reviver hook (typed as JsonReviver) to JSON reply parsing by threading it through parser.preserve and into transformRedisJsonReply/transformRedisJsonNullReply.

Updates JSON.GET, JSON.MGET, and JSON.ARRPOP to accept/pass this reviver and applies it when transforming replies (including array replies), with new specs validating date rehydration and multi() behavior.

^{Written by Cursor Bugbot for commit 5b39014. This will update automatically on new commits. Configure here.}

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[nkaradzhov]

Hi @JeanSamGirard, thanks for the PR!

I’d like 2 changes before merge:

1. JSON.ARRPOP should not require path just to pass reviver.
   Right now, passing { reviver } without path builds invalid
   args.
2. Please add tests for the new behavior:

• JSON.GET with reviver
• JSON.MGET with reviver
• JSON.ARRPOP with reviver
• one multi/pipeline case to confirm preserve flows correctly
  there too

After that, this looks good to merge.

P.S. Its generally a good idea to open an issue before any work is done. This way you will make sure the feature is aligned and accepted by the maintainers beforehand, otherwise you are risking investing the time to implement something that could be rejected later.

[JeanSamGirard]

Hi @JeanSamGirard, thanks for the PR!

I’d like 2 changes before merge:

1. JSON.ARRPOP should not require path just to pass reviver.
   Right now, passing { reviver } without path builds invalid
   args.
2. Please add tests for the new behavior:

• JSON.GET with reviver
• JSON.MGET with reviver
• JSON.ARRPOP with reviver
• one multi/pipeline case to confirm preserve flows correctly
  there too

After that, this looks good to merge.

P.S. Its generally a good idea to open an issue before any work is done. This way you will make sure the feature is aligned and accepted by the maintainers beforehand, otherwise you are risking investing the time to implement something that could be rejected later.

Hi,

I made the fix for JSON.ARRPOP and added a test using a reviver for the 3 commands. (I'm not sure in which file the test for multi should go)

Unfortunately it seems I'm unable to run them on my local for some reason... I'm on Windows 11 with docker desktop I keep getting "before all" hook for "client.json.get" I think the client isn't able to connect to the container.

PS: Noted in the future I'll make an issue first ;)

PPS: Sorry for all the additional commits, couldn't test locally so I had to trust 😅

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[JeanSamGirard]

Hi @nkaradzhov, wasn't sure if I should ping in my previous message.

I ended up adding the multi() test case in packages\json\lib\commands\GET.spec.ts using multi().json.get for the test.

If there's no other changes needed could you please approve so the tests can run?
I would've run the test locally if I could, but I can't get it to work.

Thank you