fix: resolve 4 bugs in termui - #3488
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe PR updates four independent areas: whitespace-only paragraph parsing, a clear-form keyboard condition, progress percentage rounding, and ChangesChat paragraph parsing
Form keyboard handling
Todo progress rounding
Form rejection logging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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.5)packages/ui/src/Form.tsFile contains syntax errors that prevent linting: Line 142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@examples/forms-and-validation/src/index.tsx`:
- Line 125: Update the clear-form shortcut condition in the keyboard event
handler to use a negated Ctrl check (`!event.ctrl`) rather than the postfix
assertion on `event.ctrl`, so plain “c” clears the form while Ctrl+C remains
excluded.
In `@examples/todo-app/src/index.ts`:
- Line 107: Update the percentage calculation in the progress conversion around
pct so rounding uses a tolerance appropriate to the percentage scale, preventing
boundary values such as value = 0.145 from becoming 14% instead of 15%. Add a
boundary test covering value = 0.145 and verify the expected 15% result.
In `@packages/ui/src/Form.ts`:
- Around line 141-142: Remove the trailing .catch(err =>
console.error("Promise.all failed:", err)) after submit() in Form.ts, leaving
the Promise.all handling syntactically within the submit() implementation and
ensuring no catch remains outside the function or file scope.
🪄 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: 9fff19b2-fae3-4c94-a076-ae2d508c8cd2
📒 Files selected for processing (4)
examples/chat-app/src/index.tsxexamples/forms-and-validation/src/index.tsxexamples/todo-app/src/index.tspackages/ui/src/Form.ts
| } | ||
|
|
||
| if (event.key === 'c' && event.ctrl === false) { | ||
| if (event.key === 'c' && event.ctrl !) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
if grep -nF "event.key === 'c' && event.ctrl !" "$file"; then
echo "Invalid clear-form condition remains."
exit 1
fi
grep -nF "event.key === 'c' && !event.ctrl" "$file"Repository: Karanjot786/TermUI
Length of output: 247
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
echo "Relevant lines:"
sed -n '112,130p' "$file" | nl -ba -v112
echo
echo "All Ctrl/C shortcuts in file:"
rg -n "event\.key === 'c'|ctrl|Ctrl|clear" "$file"
echo
echo "Behavioral check of asserted condition semantics (JS equivalent):"
node - <<'JS'
const cases = [
{ key: 'c', ctrl: true },
{ key: 'c', ctrl: false },
{ key: 'C', ctrl: false },
];
function currentCondition(event) {
return event.key === 'c' && Boolean(event.ctrl);
}
function correctCondition(event) {
return event.key === 'c' && !event.ctrl;
}
for (const event of cases) {
console.log(JSON.stringify(event), "current=", currentCondition(event), "correct=", correctCondition(event));
}
JSRepository: Karanjot786/TermUI
Length of output: 212
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
echo "Relevant lines:"
sed -n '112,130p' "$file" | cat -n
echo
echo "All Ctrl/C shortcuts in file:"
rg -n "event\.key === 'c'|ctrl|Ctrl|clear" "$file"
echo
echo "Behavioral check of asserted condition semantics (JS equivalent):"
node - <<'JS'
const cases = [
{ key: 'c', ctrl: true },
{ key: 'c', ctrl: false },
{ key: 'C', ctrl: false },
];
function currentCondition(event) {
return event.key === 'c' && Boolean(event.ctrl);
}
function correctCondition(event) {
return event.key === 'c' && !event.ctrl;
}
for (const event of cases) {
console.log(JSON.stringify(event), 'current=', currentCondition(event), 'correct=', correctCondition(event));
}
JSRepository: Karanjot786/TermUI
Length of output: 1756
Use !event.ctrl for the clear-form shortcut.
Line [125] uses event.ctrl !, which is a TypeScript postfix non-null assertion and preserves the Ctrl state. Handle plain c by checking !event.ctrl; otherwise Ctrl+C returns before the clear-form branch can run.
Proposed fix
- if (event.key === 'c' && event.ctrl !) {
+ if (event.key === 'c' && !event.ctrl) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (event.key === 'c' && event.ctrl !) { | |
| if (event.key === 'c' && !event.ctrl) { |
🤖 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/forms-and-validation/src/index.tsx` at line 125, Update the
clear-form shortcut condition in the keyboard event handler to use a negated
Ctrl check (`!event.ctrl`) rather than the postfix assertion on `event.ctrl`, so
plain “c” clears the form while Ctrl+C remains excluded.
Source: Coding guidelines
| const filled = Math.round(barWidth * value); | ||
|
|
||
| const pct = Math.round(value * 100); | ||
| const pct = Math.round(value * 100 + Number.EPSILON); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
test "$(node -p 'Math.round(0.145 * 100 + Number.EPSILON)')" = "14"Repository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "File exists and relevant occurrences:"
wc -l examples/todo-app/src/index.ts
rg -n "Math.round\\(|cand|pct|toFixed|EPSILON" examples/todo-app/src/index.ts || true
echo
echo "Relevant source section:"
sed -n '80,120p' examples/todo-app/src/index.tsRepository: Karanjot786/TermUI
Length of output: 2424
Use a scale-aware rounding tolerance.
Line 107 adds Number.EPSILON after value * 100, so values like 0.145 * 100 can round down to 14% instead of 15%. Add a percentage-scale tolerance or correct the value before multiplication if that matches the intended rule. Include value = 0.145 in a boundary test.
🤖 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/todo-app/src/index.ts` at line 107, Update the percentage
calculation in the progress conversion around pct so rounding uses a tolerance
appropriate to the percentage scale, preventing boundary values such as value =
0.145 from becoming 14% instead of 15%. Add a boundary test covering value =
0.145 and verify the expected 15% result.
|
|
||
| .catch(err => console.error("Promise.all failed:", err)); No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
npx --no-install biome check packages/ui/src/Form.tsRepository: Karanjot786/TermUI
Length of output: 667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a '^Form\.ts$' . | sed 's#^\./##'
wc -l packages/ui/src/Form.ts
sed -n '1,180p' packages/ui/src/Form.tsRepository: Karanjot786/TermUI
Length of output: 6721
🌐 Web query:
TypeScript trailing member dot property access error identifier expected file invalid expression parse error
💡 Result:
In TypeScript, encountering an "Identifier expected" error (often TS1003) in the context of trailing member or dot property access typically indicates a syntax mismatch where the parser requires a valid property name (an identifier) but encounters an unexpected token [1][2]. Common causes for this error include: 1. Improper Optional Chaining Syntax: The optional chaining operator must be written as ?.. If you accidentally include a space between the question mark and the dot (e.g., ?. ), the parser will treat the ? as a ternary operator or an incomplete token, leading to syntax errors like "Identifier expected" or "Expression expected" [3][4]. 2. Invalid Property Access After Instantiation: You cannot immediately follow an instantiation expression (a generic call with type arguments) with a property access in certain contexts without wrapping the expression in parentheses [5]. For example, Foo<T>.Bar() is invalid; use (Foo<T>).Bar() instead [5]. 3. Missing or Extra Delimiters: If you have an object or array literal, a missing comma between properties or elements can cause the parser to misinterpret the next token as part of the previous one, failing to find the identifier it expects for the new member [2]. 4. Invalid Use of Reserved Keywords: Using reserved keywords (e.g., class, function, let) as property names without quoting them (e.g., obj.class vs obj['class']) will trigger this error [2]. 5. Tooling/Plugin Limitations: Sometimes, third-party plugins (such as styled-component or linting plugins) may have difficulty parsing specific expressions or template literals following a dot, resulting in false-positive "Identifier expected" errors [6]. To debug this: - Verify that your optional chaining operators are written as ?. with no intervening spaces [3]. - Wrap complex expressions or instantiation expressions in parentheses if you are trying to access a property on the result [5]. - Check for missing commas in object or array definitions [2]. - Temporarily disable linting or IDE plugins to determine if the error is a compiler issue or a tool-specific reporting issue [6].
Citations:
- 1: https://medium.com/@ahmadtibibi/typescript-and-ts1003-identifier-expected-2a45f75a45b2
- 2: https://adhdecode.com/articles/typescript/ts1003-identifier-expected/
- 3: Incorrect
expected expression, got '.'error microsoft/TypeScript#62003 - 4: Typescript parser error with '?. ' (questionmark and dot) prettier/prettier-vscode#1364
- 5: weswigham/TypeScript@569cdf1
- 6: identifier expected ts-styled-plugin(9999) microsoft/typescript-styled-plugin#110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat packages/ui/tsconfig.json 2>/dev/null || true
cat tsconfig.json 2>/dev/null || true
git status --short packages/ui/src/Form.tsRepository: Karanjot786/TermUI
Length of output: 625
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if command -v tsc >/dev/null 2>&1; then
tsc --project packages/ui/tsconfig.json --noEmit --pretty false | sed -n '1,80p'
else
echo "tsc command not available"
fiRepository: Karanjot786/TermUI
Length of output: 421
Remove the trailing .catch outside Form.ts.
The .catch(...) at the repository root does not attach to Promise.all; after submit() closes, the file lacks a valid top-level expression, so TypeScript reports TS1128/TS1005 parse errors and blocks the build.
🧰 Tools
🪛 Biome (2.5.5)
[error] 142-142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'.
(parse)
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 142-142: tsup build failed: Unexpected "." syntax error.
🪛 GitHub Actions: CI / build-and-test
[error] 142-142: tsup build failed: Unexpected "." at src/Form.ts:142. Command 'bun run build' exited with code 1.
🤖 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/ui/src/Form.ts` around lines 141 - 142, Remove the trailing
.catch(err => console.error("Promise.all failed:", err)) after submit() in
Form.ts, leaving the Promise.all handling syntactically within the submit()
implementation and ensuring no catch remains outside the function or file scope.
Source: Linters/SAST tools
1 participant
Description
This PR fixes real bugs found in the codebase:
Promise.all: an unhandled rejection in any input promise previously crashed silently.x === trueis equivalent tox(andx === falseto!x), and shorter to read.Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).trim()to''misses whitespace-only input;.trim().length === 0is explicit.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3487
Summary by CodeRabbit