Skip to content

feat: Vendor prod dependency on json-stringify-safe#2910

Merged
mikicho merged 4 commits into
nock:betafrom
svix-mman:mendy/remove-dep-on-json-stringify-safe
Feb 27, 2026
Merged

feat: Vendor prod dependency on json-stringify-safe#2910
mikicho merged 4 commits into
nock:betafrom
svix-mman:mendy/remove-dep-on-json-stringify-safe

Conversation

@svix-mman
Copy link
Copy Markdown

@svix-mman svix-mman commented Sep 29, 2025

The nock project had 2 dependencies (In the beta branch)

  • @mswjs/interceptors: The network interception library
  • json-stringify-safe: A 30 line json stringify library.

This PR proposes to replace the prod dependency on json-stringify-safe, with a vendored copy.
The modifications I added ate formatting and linting

The json-stringify-safe library is only 30 lines, and is permissively licensed (ISC).

The goal here is to reduce the number of NPM packages I transiently depend on, and this one seems like a low hanging fruit.
The source for json-stringify-safe has not changed for the last 10 years, so I feel confident that we won't miss impotent updates by vendoring this dependency

@svix-mman svix-mman marked this pull request as ready for review September 29, 2025 15:26
Comment thread lib/interceptor.js Outdated
…copy

The `nock` project had 2 dependencies
- `@mswjs/interceptors`: The network interception library
- `json-stringify-safe`: A 30 line json stringify library.

This commit replaced the prod dependency on `json-stringify-safe`, with a vendored copy.
The modifications I added ate formatting and linting
@svix-mman svix-mman force-pushed the mendy/remove-dep-on-json-stringify-safe branch from 7c867ee to 39f858d Compare September 29, 2025 15:52
Copy link
Copy Markdown
Member

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it. I did not check if json-stringify-safe has unit tests, which you did not integrated. Anyhow it is not important. It only makes sense to vendorize it as is, if you need all Features. If not, i actually recommend to remove branches, which you dont need.

stringify is called always with one parameter, so second and third parameter are unused. So you remove those branches, and have a straight forward solution.

@Uzlopak
Copy link
Copy Markdown
Member

Uzlopak commented Sep 29, 2025

@svix-mman
I pushed into your PR the trimmed down implementation.

@Uzlopak Uzlopak dismissed their stale review September 29, 2025 19:41

I modified this PR.

@svix-mman
Copy link
Copy Markdown
Author

@svix-mman I pushed into your PR the trimmed down implementation.

Looks good to me 👍. Thanks for writing the test suite 😅

Please let me know if you need anything else on my end.

@Uzlopak
Copy link
Copy Markdown
Member

Uzlopak commented Sep 29, 2025

@svix-mman

fun fact... i let copilot and chatGPT create the tests.. lol

@svix-mman
Copy link
Copy Markdown
Author

@Uzlopak I guess I was too late for v15.0.0 😅

@Uzlopak
Copy link
Copy Markdown
Member

Uzlopak commented Sep 29, 2025

@svix-mman

Well, actually i wanted to merge main into beta. there was a merge conflict. So i tried to solve it in vcode, and for some reason i basically pushed on to main. Well, we have now nock 15 🚀

@mikicho
Copy link
Copy Markdown
Member

mikicho commented Oct 23, 2025

@svix-mman Thanks!
@Uzlopak can we merge this into main?

@mikicho mikicho merged commit ae85b3a into nock:beta Feb 27, 2026
14 checks passed
@mikicho
Copy link
Copy Markdown
Member

mikicho commented Feb 27, 2026

thanks!

@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 15.0.0-beta.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants