-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
adds freezeCoreModules configuration option to mitigate memory leaks #8331
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -367,6 +367,22 @@ You can collect coverage from those files with setting `forceCoverageMatch`. | |
} | ||
``` | ||
|
||
### `freezeCoreModules` [boolean] | ||
|
||
Default: `false` | ||
|
||
Prevents overriding node's core module methods so to prevent memory leaks. | ||
|
||
Attempt to override such a property will fail silently. | ||
|
||
Running with [`verbose`](#verbose-boolean) flag will enable informative print outs. | ||
|
||
### `freezeCoreModulesWhitelist` [Array<string>] | ||
|
||
Default: `['crypto']` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proxies have issues with bound native methods, hence |
||
|
||
Array of node core modules which will not be frozen. | ||
|
||
### `globals` [object] | ||
|
||
Default: `{}` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ import jestMock = require('jest-mock'); | |
import HasteMap = require('jest-haste-map'); | ||
import {formatStackTrace, separateMessageFromStack} from 'jest-message-util'; | ||
import Resolver = require('jest-resolve'); | ||
import {createDirectory, deepCyclicCopy} from 'jest-util'; | ||
import {ErrorWithStack, createDirectory, deepCyclicCopy} from 'jest-util'; | ||
import {escapePathForRegex} from 'jest-regex-util'; | ||
import Snapshot = require('jest-snapshot'); | ||
import { | ||
|
@@ -84,6 +84,7 @@ class Runtime { | |
|
||
private _cacheFS: CacheFS; | ||
private _config: Config.ProjectConfig; | ||
private _coreModulesProxyCache: {[moduleName: string]: any}; | ||
private _coverageOptions: ShouldInstrumentOptions; | ||
private _currentlyExecutingModulePath: string; | ||
private _environment: JestEnvironment; | ||
|
@@ -125,6 +126,7 @@ class Runtime { | |
collectCoverageFrom: [], | ||
collectCoverageOnlyFrom: null, | ||
}; | ||
this._coreModulesProxyCache = Object.create(null); | ||
this._currentlyExecutingModulePath = ''; | ||
this._environment = environment; | ||
this._explicitShouldMock = Object.create(null); | ||
|
@@ -776,7 +778,46 @@ class Runtime { | |
return this._environment.global.process; | ||
} | ||
|
||
return require(moduleName); | ||
const module = require(moduleName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might want to check |
||
|
||
if ( | ||
!this._config.freezeCoreModules || | ||
this._config.freezeCoreModulesWhitelist.includes(moduleName) | ||
) { | ||
return module; | ||
} | ||
|
||
if (this._coreModulesProxyCache[moduleName]) { | ||
return this._coreModulesProxyCache[moduleName]; | ||
} | ||
|
||
const set = ( | ||
target: object, | ||
property: PropertyKey, | ||
value: any, | ||
receiver: any, | ||
) => { | ||
if (typeof value !== 'function' || value._isMockFunction) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add |
||
return Reflect.set(target, property, value, receiver); | ||
} | ||
|
||
if (this._config.verbose) { | ||
console.warn( | ||
new ErrorWithStack( | ||
`Trying to override method '${property.toString()}' of a frozen core module '${moduleName}'`, | ||
set, | ||
), | ||
); | ||
} | ||
|
||
return true; | ||
}; | ||
|
||
const proxy = new Proxy(module, {set}); | ||
|
||
this._coreModulesProxyCache[moduleName] = proxy; | ||
|
||
return proxy; | ||
} | ||
|
||
private _generateMock(from: Config.Path, moduleName: string) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,8 @@ export type DefaultOptions = { | |
expand: boolean; | ||
filter: Path | null | undefined; | ||
forceCoverageMatch: Array<Glob>; | ||
freezeCoreModules: boolean; | ||
freezeCoreModulesWhitelist: Array<string>; | ||
globals: ConfigGlobals; | ||
globalSetup: string | null | undefined; | ||
globalTeardown: string | null | undefined; | ||
|
@@ -150,6 +152,8 @@ export type InitialOptions = Partial<{ | |
findRelatedTests: boolean; | ||
forceCoverageMatch: Array<Glob>; | ||
forceExit: boolean; | ||
freezeCoreModules?: boolean; | ||
freezeCoreModulesWhitelist?: Array<string>; | ||
json: boolean; | ||
globals: ConfigGlobals; | ||
globalSetup: string | null | undefined; | ||
|
@@ -386,6 +390,8 @@ export type ProjectConfig = { | |
extraGlobals: Array<keyof NodeJS.Global>; | ||
filter: Path | null | undefined; | ||
forceCoverageMatch: Array<Glob>; | ||
freezeCoreModules: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not quite sure if these should be on |
||
freezeCoreModulesWhitelist: Array<string>; | ||
globalSetup: string | null | undefined; | ||
globalTeardown: string | null | undefined; | ||
globals: ConfigGlobals; | ||
|
@@ -424,6 +430,7 @@ export type ProjectConfig = { | |
transformIgnorePatterns: Array<Glob>; | ||
watchPathIgnorePatterns: Array<string>; | ||
unmockedModulePathPatterns: Array<string> | null | undefined; | ||
verbose: boolean | null | undefined; | ||
}; | ||
|
||
export type Argv = Arguments< | ||
|
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.
our > 10K test suite runs successfully with these changes, so i guess that in most cases, there is no actual need/use of these overrides.