Storyshots is leaking memory

[SimenB]

Bug or support request summary

Bug

Steps to reproduce

See https://github.com/SimenB/storyshots-leak for example, but every single installation of storyshots (at least for React, haven't tested others) should reproduce without issue following the steps in the linked README.

To satisfy template:

1. Install and setup storyshots
2. Run jest
3. See leakage either through Jest's builtin capabilities (logHeapUsage and detectLeaks) or using some OS tool to see memory usage.

Please specify which version of Storybook and optionally any affected addons that you're running

"@storybook/addon-storyshots": "^3.3.15",
"@storybook/react": "^3.3.15",

Affected platforms

• Jest@22.4.3

Screenshots / Screencast / Code Snippets (Optional)

(A bonus would be to remove the info => Loading custom .babelrc text...)

[jrmurad]

I think we have narrowed down where the leak is. Our project has 198 test suites and we see large increase for each one with node --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage. When stubbing out initStoryshots with no-op test, the leak is ~45mb per test suite.

We eliminated that (and had tests running same as usual) by commenting out lines in node_modules/@storybook/core/server.js:

const defaultWebpackConfig = require('./dist/server/preview/base-webpack.config');
const serverUtils = require('./dist/server/utils/template');
//const buildStatic = require('./dist/server/build-static');
//const buildDev = require('./dist/server/build-dev');
const toRequireContext = require('./dist/server/preview/to-require-context');

const managerPreset = require.resolve('./dist/server/manager/manager-preset');

module.exports = {
  managerPreset,
  ...defaultWebpackConfig,
  //...buildStatic,
  //...buildDev,
  ...serverUtils,
  ...toRequireContext,
};

We're on 5.3.14.

Before (machine has 64gb RAM):

node --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage
 PASS  a (9.709s, 252 MB heap size)
 PASS  b (5.631s, 362 MB heap size)
 PASS  c (437 MB heap size)
...
 PASS  x (1937 MB heap size)
 PASS  y (5.015s, 1962 MB heap size)
 RUNS  z

<--- Last few GCs --->

[14445:0x5127f60]   117729 ms: Mark-sweep 1986.7 (2074.5) -> 1973.7 (2076.5) MB, 466.6 / 0.8 ms  (average mu = 0.246, current mu = 0.227) allocation failure scavenge might not succeed
[14445:0x5127f60]   118383 ms: Mark-sweep 1989.2 (2077.5) -> 1974.9 (2079.5) MB, 467.9 / 0.7 ms  (average mu = 0.264, current mu = 0.285) allocation failure scavenge might not succeed


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x145ce19]
Security context: 0x271be7b40921 <JSObject>
    1: _execModule [0x3187af0ba299] [.../node_modules/@jest/core/node_modules/jest-runtime/build/index.js:~936] [pc=0x64bf4aa52dc](this=0x1a8368f186e1 <Runtime map = 0x1c6ac7f12d21>,0x17f756364109 <Object map = 0x9dbcc75c5f1>,0x213632f004b9 <undefined>,0x1a8368f18a61 <Map map = 0x357ddfac0f31>,0x31ee342896e1 <String[#98]: ...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

After:

node --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage
 PASS  a (9.377s, 241 MB heap size)
 PASS  b (5.489s, 225 MB heap size)
 PASS  c (294 MB heap size)
...

[anuraags]

This is now causing issues for our workflow as well. We were unable to upgrade our storybook dependencies (including storyshots) from version 5.2.6 to 5.3.18 as our jest run would just fail with memory allocation errors.

[bastiRe]

We had the same problem and fixed it for us by separating our storyshots by folder into different testsuites. We accomplished this by creating separate Storyshot files and using the storyKindRegex to only snapshot a specific subset of stories in each file.

[keverw]

I've ran into this issue too with Storybook. Had my storybook open for a few days working on my own design system and it ended up eating up 19GB of ram (well my computer doesn't have that much ram so of course it swapping). On a Mac with the latest OS updates and using the MDX plugin too.

[vidal7]

I've ran into this issue too and while waiting for the fix, we fixed it by mocking @storybook/core/server. This solution was inspired by #3286 (comment) Thank you @jrmurad

jest.mock('@storybook/core/server', () => ({ toRequireContext: require('@storybook/core/dist/server/preview/to-require-context').toRequireContext }));

Also here is a great article that helped me a lot finding memory leaks : https://chanind.github.io/javascript/2019/10/12/jest-tests-memory-leak.html

[shilman]

@vidal7 do you know what the proper storyshots fix would be for that?

[vidal7]

@shilman, I think so. Maybe just changing in this file (addons/storyshots/storyshots-core/src/frameworks/configure.ts) a import. So

Change
import { toRequireContext } from '@storybook/core/server';

To
const toRequireContext = require('../../../core/dist/server/preview/to-require-context');

I think it will work. The problem is that importing toRequireContext from '@storybook/core/server'; imports too much things in addons/storyshots/storyshots-core/src/frameworks/configure.js. Because of typescript compilation to javascript, the first import is compiled to var server_1 = require("@storybook/core/server"); in @storybook/addon-storyshots/dist/frameworks/configure.js

[shilman]

Oof that's pretty ugly though. @ndelangen @tmeasday WDYT about a little refactoring here?

[ndelangen]

This might work:

import { toRequireContext } from '@storybook/core/dist/server/preview/to-require-context';

?

[stu039]

+1

[rajanlagah]

It is eating every resource of the PC here. 😢

[rajanlagah]

Do we think running all these node processes in docker by limiting resources will help ?
I think if we limit number of CPU for yarn dev command then it will take time but will be in control of developer.
I can help building this have some experience with running node apps in docker.

[dheerajk02]

I've ran into this issue too and while waiting for the fix, we fixed it by mocking @storybook/core/server. This solution was inspired by #3286 (comment) Thank you @jrmurad

jest.mock('@storybook/core/server', () => ({ toRequireContext: require('@storybook/core/dist/server/preview/to-require-context').toRequireContext }));

Also here is a great article that helped me a lot finding memory leaks : https://chanind.github.io/javascript/2019/10/12/jest-tests-memory-leak.html

@vidal7
where did you exactly add this line
jest.mock('@storybook/core/server', () => ({ toRequireContext: require('@storybook/core/dist/server/preview/to-require-context').toRequireContext }));

is it inside the index.spec.js before the initStoryshot()?

[vidal7]

@dheerajk02 , in a jest setup file

https://jestjs.io/docs/configuration#setupfiles-array

[ndelangen]

It's been changed to

storybook/code/addons/storyshots-core/src/frameworks/configure.ts

Line 11 in af0578d

import { toRequireContext } from '@storybook/core-webpack';

Which -I think- solved the issue..