fix(jsx): log unmount errors instead of silently swallowing them

[ZainabTravadi]

Description

Fixes a bug in @termuijs/jsx where errors thrown during app.unmount() were silently swallowed during cleanup.

This change preserves the existing non-throwing cleanup behavior while logging unmount failures for improved observability and debugging. A regression test has been added to verify the behavior.

Related Issue

Closes #1757

Which package(s)?

@termuijs/jsx

Type of Change

• 🐛 Bug fix (type:bug)
• 🧪 Tests (type:testing)

Checklist

• ⭐ You starred the repo. The needs-star check blocks your merge otherwise.
• Tests pass locally: bun vitest run
• Build passes: bun run build
• Typecheck passes: bun run typecheck
• You read CONTRIBUTING.md.
• Your PR title follows type: short description.
• Widget state mutators call markDirty() (if your change affects rendering).
• No new any types without an inline comment explaining why.
• No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

• You are a GSSoC 2026 contributor.
• Your GSSoC profile: https://gssoc.girlscript.org/profile/3652c85d-e890-41f5-a48f-584ed0a7f209

Screenshots / Recordings (UI changes)

N/A

Notes for the Reviewer

Previously, unmount errors were caught and ignored using an empty catch block, making cleanup failures difficult to diagnose.

This PR:

• Preserves the existing cleanup flow

• Logs unmount errors for observability

• Adds a regression test ensuring:

  • cleanup continues after an unmount failure
  • errors do not escape
  • logging occurs exactly once with the expected message

Summary by CodeRabbit

• Bug Fixes

  • Improved cleanup reliability by handling errors during app unmount and logging a clear, single error message instead of failing silently.

• New Features

  • Exposed a public unmountApps utility for reliably unmounting multiple apps.

• Tests

  • Added a test suite covering error scenarios to ensure unmount continues for subsequent apps and that logging occurs as expected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1058e02b-f44b-4bbf-9e22-696445626cde

📥 Commits

Reviewing files that changed from the base of the PR and between 122577f and 8976116.

📒 Files selected for processing (1)

• packages/jsx/src/unmountHelpers.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/jsx/src/unmountHelpers.test.ts

---

📝 Walkthrough

Walkthrough

The private _unmountApps helper in render.ts is replaced with an exported unmountApps function that catches errors from app.unmount() and logs them via console.error instead of silently discarding them. The HMR dispose call site is updated to use the new function. A new Vitest test suite validates the error-logging and continuation behavior.

Changes

unmountApps error logging and export

Layer / File(s)	Summary
unmountApps implementation, export, and HMR call site packages/jsx/src/render.ts	Replaces _unmountApps (private, silent catch) with exported unmountApps that calls console.error with a [jsx] Error during unmount(): prefix on failure. The HMR dispose fallback at line 169 is updated to call unmountApps(apps).
unmountApps test suite packages/jsx/src/unmountHelpers.test.ts	New Vitest suite verifies that unmountApps does not rethrow, still invokes unmount on subsequent apps after a failure, and calls console.error exactly once with the expected message and thrown error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Karanjot786/TermUI#1359: Introduced the _unmountApps fallback in the HMR dispose path inside packages/jsx/src/render.ts that this PR replaces with the exported unmountApps.

Suggested labels

quality:clean

Suggested reviewers

• Karanjot786

Poem

🐇 Hop, hop! No more silent fails,
The unmount error now tells its tales.
A console.error lights the way,
So bugs won't quietly slip away.
This bunny logs them — hip hooray! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and concisely describes the main change: converting silent error swallowing to logging unmount errors. It directly reflects the primary objective.
Description check	✅ Passed	The PR description comprehensively follows the template with all required sections filled: description, related issue, package, type of change, checklist completion, and notes for reviewer.
Linked Issues check	✅ Passed	The code changes fully satisfy the objectives from issue #1757: an exported unmountApps function now logs errors via console.error while preserving cleanup flow, and a regression test verifies the new behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to addressing the issue: exporting the unmount helper, adding error logging, and testing the new behavior. No unrelated refactoring or modifications are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/jsx/src/unmountHelpers.test.ts`:
- Around line 10-23: The spy.mockRestore() call at the end of the test only
executes if all preceding expect assertions pass, risking a leaked mock in
subsequent tests if any assertion fails first. Wrap the test logic (from the spy
creation through the expect statements) in a try-finally block to ensure
spy.mockRestore() is always called regardless of whether assertions throw
errors, guaranteeing the console.error mock is properly cleaned up after the
test completes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 402ce3c5-3125-484b-998d-74926f38e783

📥 Commits

Reviewing files that changed from the base of the PR and between 35c2213 and 122577f.

📒 Files selected for processing (2)

• packages/jsx/src/render.ts
• packages/jsx/src/unmountHelpers.test.ts

[ZainabTravadi]

Updated the test to restore the console.error spy inside a finally block so cleanup always occurs even if an assertion fails. Verified the jsx test suite still passes.