Skip to content

Commit

Permalink
fix: only activate once, when on (#1824)
Browse files Browse the repository at this point in the history
While debugging the Jest memory leak, I was surprised to find the Nock
was "activating" twice when imported. This is because intercept.js
was calling `activate()` directly _AND_ back was setting the mode, which
calls `restore()` and `activate()`. There is no need for this double
initialization. Think of the CPUs we could be saving.
I also discovered that Nock "activates" even if the proc is started with
the `NOCK_OFF` env var set, which means Nock is polluting the global
scope even when callers have asked it not to.

Since it felt like the `back.setMode()` call was hiding in its module,
I've moved it to the entry index file and wrapped it in a `isOn` check.
  • Loading branch information
mastermatt committed Feb 10, 2020
1 parent 8b8a10d commit a56a209
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 5 deletions.
11 changes: 10 additions & 1 deletion index.js
Expand Up @@ -6,6 +6,7 @@ const {
activate,
isActive,
isDone,
isOn,
pendingMocks,
activeMocks,
removeInterceptor,
Expand All @@ -31,7 +32,7 @@ Object.assign(module.exports, {
// TODO-12.x Historically `nock.cleanAll()` has returned the nock global.
// The other global methods do not do this, so it's not clear this was
// deliberate or is even helpful. This shim is included for backward
// compatibility and shoulud be replaced with an alias to `removeAll()`.
// compatibility and should be replaced with an alias to `removeAll()`.
cleanAll() {
removeAll()
return module.exports
Expand All @@ -49,3 +50,11 @@ Object.assign(module.exports, {
restore: recorder.restore,
back,
})

// We always activate Nock on import, overriding the globals.
// Setting the Back mode "activates" Nock by overriding the global entries in the `http/s` modules.
// If Nock Back is configured, we need to honor that setting for backward compatibility,
// otherwise we rely on Nock Back's default initializing side effect.
if (isOn()) {
back.setMode(process.env.NOCK_BACK_MODE || 'dryrun')
}
1 change: 0 additions & 1 deletion lib/back.js
Expand Up @@ -284,6 +284,5 @@ Back.setMode = function(mode) {

Back.fixtures = null
Back.currentMode = null
Back.setMode(process.env.NOCK_BACK_MODE || 'dryrun')

module.exports = Back
3 changes: 0 additions & 3 deletions lib/intercept.js
Expand Up @@ -428,8 +428,6 @@ function activate() {
})
}

activate()

module.exports = {
addInterceptor,
remove,
Expand All @@ -443,7 +441,6 @@ module.exports = {
activeMocks,
enableNetConnect,
disableNetConnect,
overrideClientRequest,
restoreOverriddenClientRequest,
abortPendingRequests: common.removeAllTimers,
}
1 change: 1 addition & 0 deletions tests/test_back.js
Expand Up @@ -502,6 +502,7 @@ test('assertScopesFinished throws exception when Back still has pending scopes',

test('nockBack dryrun throws the expected exception when fs is not available', t => {
const nockBackWithoutFs = proxyquire('../lib/back', { fs: null })
nockBackWithoutFs.setMode('dryrun')

nockBackWithoutFs.fixtures = `${__dirname}/fixtures`
t.throws(() => nockBackWithoutFs('goodRequest.json'), { message: 'no fs' })
Expand Down
12 changes: 12 additions & 0 deletions tests/test_nock_off.js
@@ -1,6 +1,7 @@
'use strict'

const { expect } = require('chai')
const http = require('http')
const nock = require('..')
const got = require('./got_client')
const ssl = require('./ssl')
Expand Down Expand Up @@ -41,4 +42,15 @@ describe('NOCK_OFF env var', () => {
expect(body).to.equal(responseBody)
scope.done()
})

it('when true before import, Nock does not activate', async () => {
nock.restore()
const originalClient = http.ClientRequest

delete require.cache[require.resolve('..')]
const newNock = require('..')

expect(http.ClientRequest).to.equal(originalClient)
expect(newNock.isActive()).to.equal(false)
})
})

0 comments on commit a56a209

Please sign in to comment.