Skip to content

fix(eslint-plugin-fiori-tools): fix heap OOM on macos Node 20 arm64#4570

Open
longieirl wants to merge 11 commits intomainfrom
fix/4545-eslint-plugin-oom-v2
Open

fix(eslint-plugin-fiori-tools): fix heap OOM on macos Node 20 arm64#4570
longieirl wants to merge 11 commits intomainfrom
fix/4545-eslint-plugin-oom-v2

Conversation

@longieirl
Copy link
Copy Markdown
Contributor

Root cause

The previous fix (#4566) added --merge-async to c8, targeting coverage merge time. The full stack trace reveals the OOM is one layer deeper — in Node's native V8CoverageConnection::WriteProfile at process exit:

RunAtExitCallbacks → StartProfilers → takePreciseCoverage → JsonStringifier → OOM

c8 runs as a single orchestrator process that accumulates V8 profiler data for all 71 test files before serialising it. With --experimental-vm-modules, each test file re-instruments every source module, producing ~269 MB of raw coverage data. JSON.stringify of this object requires a contiguous string buffer that exhausts the ~2 GB heap on macos arm64 Node 20. --merge-async only affects how c8 handles already-written JSON files — the OOM fires before that step.

Fix

Replace the c8 process-level wrapper with Jest's built-in coverageProvider: 'v8'. Jest's own V8 provider collects coverage per worker process (one test file each) via collect-v8-coverage, keeping per-process memory bounded. The main Jest process aggregates small per-worker reports — the large serialisation never happens in a single process.

Also removes the unused c8 devDependency and adds --maxWorkers=50% (scales to available CPUs rather than hardcoding 2).

--experimental-vm-modules is still required — ESLint 9 uses dynamic import() internally and fails without it.

Changes

  • jest.config.js: remove collectCoverage: false, add coverageProvider: 'v8'
  • package.json: drop c8 wrapper, keep NODE_OPTIONS='--experimental-vm-modules', add --maxWorkers=50%

Test plan

  • CI passes on macos-latest Node 20.x without OOM
  • 583 tests pass (verified locally)
  • Coverage output unchanged in quality

Closes #4545

@longieirl longieirl requested a review from a team as a code owner April 17, 2026 10:41
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 17, 2026

⚠️ No Changeset found

Latest commit: b68feeb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown
Contributor

@hyperspace-insights hyperspace-insights Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is a clean, well-motivated fix — removing the c8 process-level wrapper in favour of Jest's built-in coverageProvider: 'v8' is the correct solution for the per-process OOM described. The only suggestion is to add a brief comment in jest.config.js making the implicit collectCoverage: true inheritance explicit, to guard against future regressions.

PR Bot Information

Version: 1.20.11 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Agent Instructions:
  • LLM: anthropic--claude-4.6-sonnet
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • Correlation ID: 408de502-245b-4200-bafb-3b7e04808e01

Comment thread packages/eslint-plugin-fiori-tools/jest.config.js
@longieirl longieirl self-assigned this Apr 17, 2026
@longieirl
Copy link
Copy Markdown
Contributor Author

@heimwege @devinea please review

Comment thread .changeset/eslint-plugin-fiori-tools-oom-fix-v2.md Outdated
Comment thread packages/eslint-plugin-fiori-tools/package.json
@longieirl longieirl requested review from devinea and heimwege April 21, 2026 09:37
Copy link
Copy Markdown
Contributor

@heimwege heimwege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, let's give it a try (also because I'm getting annoyed by having to start the runs over and over again because of the heap issue 🤣 )

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG - Out of memory crash on Node 20 arm64 after test suite completes

3 participants