Skip to content

fix: resolve 4 bugs in termui - #3529

Closed
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-45972
Closed

fix: resolve 4 bugs in termui#3529
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-45972

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes real bugs found in the codebase:

  • Added explicit radix to parseInt: without 10, strings like '0x1F' or '08' parse in unintended bases.
  • Prevented interval leak: repeated mounts now clear the previous interval before scheduling a new one.
  • Simplified empty-string validation: comparing trim() to '' misses whitespace-only input; .trim().length === 0 is explicit.
  • Filled empty catch block: silently swallowing the error hides failures; now logs for debugging.

Type of Change

  • Bug fix (non-breaking change fixing an issue)

How Has This Been Tested?

  • Local manual testing

Checklist

  • My code follows the style guidelines
  • I have performed a self-review

Related Issue

Ref: #3528

Summary by CodeRabbit

  • Bug Fixes
    • Prevented overlapping streaming timers, improving consistency during streamed text updates.
    • Improved chat content parsing so whitespace-only lines are handled as blank paragraphs.
    • Made showcase tab-key number handling more reliable.
    • Improved development server reload diagnostics by reporting child-process errors instead of silently ignoring them.

@github-actions github-actions Bot added type:bug +10 pts. Bug fix. area:examples Example apps. area:dev-server @termuijs/dev-server labels Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The changes update streaming timer cleanup, whitespace-only paragraph parsing, tab-key radix handling, and reload exit error logging. The reload handler change contains a syntactically invalid catch expression.

Changes

Streaming timer update

Layer / File(s) Summary
Streaming interval reset
examples/ai-streaming/src/index.tsx
The constructor clears the stored interval before creating a new 50 ms streaming interval.

Example parsing updates

Layer / File(s) Summary
Whitespace and radix parsing
examples/chat-app/src/index.tsx, examples/showcase/src/index.tsx
parseBlocks treats whitespace-only lines as empty paragraphs. ShowcaseApp.handleKey parses tab values with radix 10.

Reload error handling

Layer / File(s) Summary
Reload exit error handler
packages/dev-server/src/server.ts
The reload flow changes the exitedPromise catch expression to log errors. The expression is syntactically invalid.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: karanjot786

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the fixes and testing, but it omits the required package section and uses a reference instead of a closing issue link. Add the affected package names, change Ref: #3528 to `Closes `#3528, and complete the required checklist and GSSoC sections.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the change as a bug fix and summarizes the four resolved bugs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Biome (2.5.6)
packages/dev-server/src/server.ts

File contains syntax errors that prevent linting: Line 383: Expected a parenthesis '(' but instead found '=>'.


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

Comment @coderabbitai help to get the list of available commands.

@saurabhhhcodes saurabhhhcodes mentioned this pull request Aug 5, 2026
4 tasks

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
examples/ai-streaming/src/index.tsx (1)

43-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Keep the streaming interval on the AIStreamingApp instance.

window.__interval is the only shared owner, so current usage does not show duplicate timers. However, clearInterval(window.__interval) still cancels that global slot on remounts if this app can mount again. Use instance-owned timer state with teardown cleanup, following examples/widget-gallery/src/tabs/ai-tab.ts.

🤖 Prompt for 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.

In `@examples/ai-streaming/src/index.tsx` at line 43, Update AIStreamingApp to
store the streaming interval on the component instance rather than
window.__interval, and clear that instance-owned timer during teardown or
unmount. Follow the timer ownership and cleanup pattern used by ai-tab.ts while
preserving the existing streaming behavior.
🤖 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/dev-server/src/server.ts`:
- Line 383: Fix the malformed catch callback on exitedPromise by providing a
valid callback parameter and logging the caught error through console.error.
Preserve the existing await and promise-rejection handling behavior.

---

Nitpick comments:
In `@examples/ai-streaming/src/index.tsx`:
- Line 43: Update AIStreamingApp to store the streaming interval on the
component instance rather than window.__interval, and clear that instance-owned
timer during teardown or unmount. Follow the timer ownership and cleanup pattern
used by ai-tab.ts while preserving the existing streaming behavior.
🪄 Autofix

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: 1275b341-403c-4a94-9ab0-377f20c53e0f

📥 Commits

Reviewing files that changed from the base of the PR and between 6c7584e and 7cb9c23.

📒 Files selected for processing (4)
  • examples/ai-streaming/src/index.tsx
  • examples/chat-app/src/index.tsx
  • examples/showcase/src/index.tsx
  • packages/dev-server/src/server.ts

this._killChild();

await exitedPromise.catch(() => {});
await exitedPromise.catch( => console.error());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

file="packages/dev-server/src/server.ts"

if rg -nP '\.catch\(\s*=>' "$file"; then
  echo "Invalid catch callback remains in $file" >&2
  exit 1
fi

Repository: Karanjot786/TermUI

Length of output: 288


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

file="packages/dev-server/src/server.ts"

echo "Relevant line:"
sed -n '370,390p' "$file" | nl -ba -v370

echo
echo "Parse/compile check with TypeScript parser if available:"
if command -v tsc >/dev/null 2>&1; then
  tsc --noEmit --strict --checkJs false "$file" 2>&1 | sed -n '1,80p' || true
else
  echo "tsc not available"
fi

echo
echo "Behavioral probe for the callback expression alone:"
node --input-type=module - <<'JS'
try {
  new Function('await new Promise(resolve => setTimeout(resolve, 1)).catch( => console.error());');
  console.log('parsed');
} catch (error) {
  console.log(error.name + ': ' + error.message);
}
JS

Repository: Karanjot786/TermUI

Length of output: 211


Fix the malformed catch callback.

await exitedPromise.catch( => console.error()); uses => without a callback parameter, so packages/dev-server/src/server.ts still fails to parse. Use an explicit callback parameter or a callback body, such as (error) => console.error(error).

🧰 Tools
🪛 Biome (2.5.6)

[error] 383-383: Expected a parenthesis '(' but instead found '=>'.

(parse)

🤖 Prompt for 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.

In `@packages/dev-server/src/server.ts` at line 383, Fix the malformed catch
callback on exitedPromise by providing a valid callback parameter and logging
the caught error through console.error. Preserve the existing await and
promise-rejection handling behavior.

Source: Linters/SAST tools

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

Labels

area:dev-server @termuijs/dev-server area:examples Example apps. type:bug +10 pts. Bug fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant