fix(twenty-sdk): stop dev-mode OOM by caching compiled manifest modules (#22435)
## Context Closes twentyhq/core-team-issues#2601 `twenty dev` crashed with a Node.js heap OOM (`FATAL ERROR: Reached heap limit — JavaScript heap out of memory`) after a while of editing. ## Root cause `loadModule()` in `manifest-extract-config-from-file.ts` compiled every manifest-defining file with **`vm.compileFunction`** on every manifest rebuild. V8 pins every function compiled through the `vm` module and never releases it ([nodejs/node#35375](https://github.com/nodejs/node/issues/35375)). In the dev loop this is on the hottest path and heavily amplified: - `runSyncPipeline` → `buildManifest` re-globs **all** `.ts/.tsx` files and recompiles every entity file on **every** sync — not just the edited one. - A single save triggers 2+ full rebuilds (the manifest watcher change → `scheduleSync`, then the esbuild watcher's `handleFileBuilt` → `scheduleSync` again). - Each compiled unit is the full esbuild bundle — hundreds of KB, up to MBs for front components (React/JSX inlined). So over an hour of editing, thousands of `vm.compileFunction` calls × large source, all permanently retained → multi-GB heap → crash. This matches the reported profile exactly. Investigation ruled out (with evidence): chokidar watchers (disposed on restart), ts-morph/`createProgram` (dead code, not in the dev loop — typecheck runs in child `tsc` processes), the event log (hard-capped at 200), Ink timers/subscriptions (all cleaned up), and graphql-sse (only used by `logs`). ## Change Keep only the **latest build per file**, keyed by file path. Each cache entry stores the file's last bundled-output hash and its compiled wrapper: - Rebuild with **unchanged** output → reuse the existing wrapper (no recompile). - Output **changed** → overwrite the entry, so the file's previous build is dropped instead of accumulating. This bounds the cache to one entry per file rather than one per rebuild, so old builds no longer pile up in the heap. The wrapper is still executed fresh into a new module shim on every call, so extraction behavior is unchanged. One file changed: `packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-extract-config-from-file.ts`. ## Test - Behavior of `extractManifestFromFile` is unchanged (fresh execution per call); only redundant recompilation is eliminated and stale builds are dropped. - Note: `yarn install` could not complete in the authoring sandbox (a git-based transitive dep of `twenty-desktop` is blocked by the proxy), so lint/typecheck/tests were not run locally — relying on CI. ## Follow-ups (not in this PR) - Redundant **double-sync per save** (`start-watchers-orchestrator-step.ts`) triggers two full rebuilds per edit. - Latent **event-log display bug**: new events stop appearing once the 200-event cap is reached.
This commit is contained in:
+92
@@ -1,5 +1,9 @@
|
||||
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
|
||||
import os from 'node:os';
|
||||
import vm from 'node:vm';
|
||||
import { createRequire } from 'module';
|
||||
import { join } from 'path';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { MINIMAL_APP_PATH } from '@/cli/__tests__/apps/fixture-paths';
|
||||
import { extractManifestFromFile } from '@/cli/utilities/build/manifest/manifest-extract-config-from-file';
|
||||
@@ -41,3 +45,91 @@ describe('extractManifestFromFile', () => {
|
||||
expect(Object.keys(requireCache).length).toBe(before);
|
||||
}, 60000);
|
||||
});
|
||||
|
||||
// Regression guard for the dev-mode OOM (twenty#2601). The manifest is rebuilt
|
||||
// on every file change and previously recompiled every entity file through
|
||||
// vm.compileFunction each time, which V8 never releases. The compiled wrapper
|
||||
// is now cached per file, so an unchanged file must not be recompiled and a
|
||||
// changed file must replace (not accumulate) its previous build. Fresh temp
|
||||
// files are used so the per-file cache is cold regardless of test order.
|
||||
describe('extractManifestFromFile compiled-module caching', () => {
|
||||
let tmpDir: string;
|
||||
|
||||
const writeConfigFile = async (
|
||||
configFilePath: string,
|
||||
config: Record<string, unknown>,
|
||||
): Promise<void> => {
|
||||
await writeFile(
|
||||
configFilePath,
|
||||
`export default ${JSON.stringify(config)};\n`,
|
||||
'utf8',
|
||||
);
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
tmpDir = await mkdtemp(join(os.tmpdir(), 'manifest-extract-cache-test-'));
|
||||
await writeFile(
|
||||
join(tmpDir, 'package.json'),
|
||||
'{ "name": "test-app" }',
|
||||
'utf8',
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
await rm(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('compiles an unchanged file only once across repeated builds', async () => {
|
||||
const configFilePath = join(tmpDir, 'unchanged-config.ts');
|
||||
const config = { name: 'unchanged' };
|
||||
await writeConfigFile(configFilePath, config);
|
||||
|
||||
const compileFunctionSpy = vi.spyOn(vm, 'compileFunction');
|
||||
|
||||
const firstResult = await extractManifestFromFile({
|
||||
filePath: configFilePath,
|
||||
appPath: tmpDir,
|
||||
});
|
||||
const secondResult = await extractManifestFromFile({
|
||||
filePath: configFilePath,
|
||||
appPath: tmpDir,
|
||||
});
|
||||
const thirdResult = await extractManifestFromFile({
|
||||
filePath: configFilePath,
|
||||
appPath: tmpDir,
|
||||
});
|
||||
|
||||
// Three rebuilds, but identical output is compiled only once.
|
||||
expect(compileFunctionSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Behaviour is unchanged: every call still returns the extracted config.
|
||||
expect(firstResult).toEqual(config);
|
||||
expect(secondResult).toEqual(config);
|
||||
expect(thirdResult).toEqual(config);
|
||||
}, 60000);
|
||||
|
||||
it('recompiles when the file output changes', async () => {
|
||||
const configFilePath = join(tmpDir, 'changing-config.ts');
|
||||
await writeConfigFile(configFilePath, { name: 'before' });
|
||||
|
||||
const compileFunctionSpy = vi.spyOn(vm, 'compileFunction');
|
||||
|
||||
const beforeResult = await extractManifestFromFile({
|
||||
filePath: configFilePath,
|
||||
appPath: tmpDir,
|
||||
});
|
||||
|
||||
await writeConfigFile(configFilePath, { name: 'after' });
|
||||
|
||||
const afterResult = await extractManifestFromFile({
|
||||
filePath: configFilePath,
|
||||
appPath: tmpDir,
|
||||
});
|
||||
|
||||
// One compile per distinct output, and the new content is returned.
|
||||
expect(compileFunctionSpy).toHaveBeenCalledTimes(2);
|
||||
expect(beforeResult).toEqual({ name: 'before' });
|
||||
expect(afterResult).toEqual({ name: 'after' });
|
||||
}, 60000);
|
||||
});
|
||||
|
||||
+46
-5
@@ -1,6 +1,7 @@
|
||||
import { conditionalAvailabilityTransformPlugin } from '@/cli/utilities/build/common/conditional-availability/conditional-availability-transform-plugin';
|
||||
import { pathExists } from '@/cli/utilities/file/fs-utils';
|
||||
import { type ValidationResult } from '@/sdk/define';
|
||||
import { createHash } from 'node:crypto';
|
||||
import * as esbuild from 'esbuild';
|
||||
import { createRequire } from 'module';
|
||||
import vm from 'node:vm';
|
||||
@@ -15,6 +16,50 @@ type CompiledModuleWrapper = (
|
||||
dirname: string,
|
||||
) => void;
|
||||
|
||||
type CachedCompiledModule = {
|
||||
outputHash: string;
|
||||
wrapper: CompiledModuleWrapper;
|
||||
};
|
||||
|
||||
// vm.compileFunction pins every function it compiles at the V8 isolate level
|
||||
// and never releases it (nodejs/node#35375). In dev mode the manifest is
|
||||
// rebuilt on every file change and recompiles every entity file, so compiling
|
||||
// the same file over and over grows the heap without bound and eventually
|
||||
// OOM-crashes the process.
|
||||
//
|
||||
// We keep only the latest build per file, keyed by file path: when a file is
|
||||
// rebuilt with unchanged output we reuse its wrapper, and when its output
|
||||
// changes we overwrite the entry so the previous build is dropped instead of
|
||||
// accumulating. This bounds the cache to one entry per file rather than one
|
||||
// per rebuild.
|
||||
const compiledModuleCacheByFilePath = new Map<string, CachedCompiledModule>();
|
||||
|
||||
const getCompiledWrapper = (
|
||||
code: string,
|
||||
filePath: string,
|
||||
): CompiledModuleWrapper => {
|
||||
const outputHash = createHash('sha1').update(code).digest('hex');
|
||||
|
||||
const cachedModule = compiledModuleCacheByFilePath.get(filePath);
|
||||
|
||||
if (isDefined(cachedModule) && cachedModule.outputHash === outputHash) {
|
||||
return cachedModule.wrapper;
|
||||
}
|
||||
|
||||
const compiledWrapper = vm.compileFunction(
|
||||
code,
|
||||
['exports', 'require', 'module', '__filename', '__dirname'],
|
||||
{ filename: filePath },
|
||||
) as unknown as CompiledModuleWrapper;
|
||||
|
||||
compiledModuleCacheByFilePath.set(filePath, {
|
||||
outputHash,
|
||||
wrapper: compiledWrapper,
|
||||
});
|
||||
|
||||
return compiledWrapper;
|
||||
};
|
||||
|
||||
const MANIFEST_MOCK_MODULES = [
|
||||
'twenty-ui',
|
||||
'twenty-client-sdk/core',
|
||||
@@ -102,11 +147,7 @@ const loadModule = async ({
|
||||
|
||||
const code = result.outputFiles[0].text;
|
||||
|
||||
const compiledWrapper = vm.compileFunction(
|
||||
code,
|
||||
['exports', 'require', 'module', '__filename', '__dirname'],
|
||||
{ filename: filePath },
|
||||
) as unknown as CompiledModuleWrapper;
|
||||
const compiledWrapper = getCompiledWrapper(code, filePath);
|
||||
|
||||
const moduleShim: { exports: Record<string, unknown> } = { exports: {} };
|
||||
|
||||
|
||||
Reference in New Issue
Block a user