fix: fix memory leak when using nock with jest - #2572

Thank you for taking on this problem! Unfortunately your change seems to have broken the Node 10 and 12 tests (I know).

Maybe only run test:jest in CI when Node version is at least 14? We will bump to Node 20 as part of fixing #2397

Hey @gr2m, just updated the job

Looks like need to fix the command for windows as well 😄

@gr2m should be ok now, checked in my repo

🎉 This PR is included in version 13.5.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

hm... the removal of

    const module = {
      http: require('http'),
      https: require('https'),
    }[moduleName]

broke my tests. Patching it back in fixed it. I didn't look into the root causes but obviously http and https need to be preloaded...

@perrin4869 could you create a minimal reproduction repository?

well of course I'd like to setup a minimal reproduction and write a regression test to make sure it doesn't break again in the future ^^;;
I expect it wouldn't be easy to get a minimal reproduction, since the code that broke actually runs inside a lambda so I cannot easily reproduce it locally, I may try today or this week

it's most likely weird lambda code bundling ... I've been bitten by this before. I'd suggest we revert this particular change and add a comment with a warning for future contributors to ... just leave it alone 😁

@gr2m that's be super helpful! I'm sure it was coded that way for a reason 😅

Thanks for the fix @VladimirChuprazov !

I'm curious whether https://www.npmjs.com/package/nock#memory-issues-with-jest is still relevant with this fix now, would you know?

@markmssd
How about you satisfy your curiosity and check if that is still relevant and report it back? ;)

Still required :)