Skip to content

fix: code quality and safety improvements #3358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saurabhhhcodes wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: code quality and safety improvements #3358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 1 addition & 22 deletions examples/cli-wrapper-live/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,4 @@ export function LiveWrapper() {
readStream();

proc.exited.then((code) => {
setStatus(`Process exited with code ${code}`);
if (buffer) {
setLines(prev => [...prev, buffer]);
}
});

return () => {
proc.kill();
};
}, []);

return (
<box width={60} height={20} border="single" flexDirection="column">
<StreamingTextWidget text={status} speed={5} style={{ fg: 'cyan', height: 1 }} />
<box flexGrow={1} border="top">
<LogViewWidget lines={lines} />
</box>
</box>
);
}

renderApp(LiveWrapper);
.catch(err => console.error(err))

@coderabbitai coderabbitai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Restore successful-exit handling and lifecycle cleanup.

A rejection handler does not run when proc.exited resolves normally. The current change therefore drops the final status update and buffered trailing line. It also removes proc.kill() on unmount, the LiveWrapper JSX return, and renderApp(LiveWrapper), so the example cannot provide its previous UI behavior and can leave the subprocess running. (github.com)

Keep the success handler, restore the effect cleanup and component return, and retain error logging as the rejection branch.

🧰 Tools
🪛 Biome (2.5.5)

[error] 67-67: Expected a statement but instead found '.catch(err => console.error(err))'.

(parse)

[error] 67-67: expected } but instead the file ends

(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 `@examples/cli-wrapper-live/src/index.tsx` at line 67, Restore the
successful-resolution handler for proc.exited so it performs the final status
update and flushes the buffered trailing line, while keeping console.error(err)
as the rejection branch. In the LiveWrapper component, restore the unmount
cleanup that calls proc.kill(), return the LiveWrapper JSX, and restore the
renderApp(LiveWrapper) entry point.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Blocker: complete the proc.exited promise chain.

Line [67] starts .catch(...) after an unclosed proc.exited.then((code) => {. The file has an unmatched { and cannot compile. The patch also leaves the callback body removed. (github.com)

Restore the callback and append .catch(...) after });, or remove the then opener entirely for rejection-only handling.

Proposed syntax fix 
-        proc.exited.then((code) => {
-        .catch(err => console.error(err))
+        proc.exited.catch(err => console.error(err));
🧰 Tools
🪛 Biome (2.5.5)

[error] 67-67: Expected a statement but instead found '.catch(err => console.error(err))'.

(parse)

[error] 67-67: expected } but instead the file ends

(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 `@examples/cli-wrapper-live/src/index.tsx` at line 67, Complete the proc.exited
promise chain by restoring the callback body and closing the
proc.exited.then((code) => { block with }); before the existing catch handler,
ensuring the file has balanced braces and compiles.

Source: Linters/SAST tools

30 changes: 1 addition & 29 deletions packages/jsx/src/lazy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,4 @@ export function lazy<TProps = any>(
};

promise = loader().then(
(mod) => {
status = 'resolved';
result = mod.default;
triggerRender();
},
(err) => {
status = 'rejected';
result = err;
triggerRender();
},
);

throw promise;
}

if (status === 'pending') {
throw promise;
}

if (status === 'rejected') {
throw result;
}

const Component = result as FC<TProps>;
return Component(props as TProps & { children?: any });
};

return LazyComponent;
}
.catch(err => console.error(err))

@coderabbitai coderabbitai Bot Aug 1, 2026

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

Restore a valid promise expression before .catch.

Line 27 starts with member access and has no left-hand expression. This causes the reported parse errors and prevents the package from compiling. Attach .catch(...) to the loader promise expression and restore the required enclosing syntax.

🧰 Tools
🪛 Biome (2.5.5)

[error] 27-27: Expected an expression but instead found '.'.

(parse)

[error] 27-27: expected } but instead the file ends

(parse)

🪛 GitHub Actions: CI / 0_build-and-test.txt

[error] 27-27: tsup/esbuild build failed: Unexpected "." at the .catch(err => console.error(err)) expression. The @termuijs/jsx#build command exited with code 1.

🪛 GitHub Actions: CI / build-and-test

[error] 27-27: tsup/esbuild build failed: Unexpected "." at the .catch(err => console.error(err)) expression. The @termuijs/jsx build command 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/jsx/src/lazy.ts` at line 27, Fix the promise chain in the lazy
loader expression around the existing .catch handler so it has a valid
loader-promise expression before member access. Restore the required enclosing
syntax and preserve the current console.error(err) rejection handling.

Source: Linters/SAST tools

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not replace rejected lazy state with logging only.

packages/jsx/src/lazy.test.ts:73-109 requires a rejected loader to schedule a render and throw the loader error on the next render. Logging the rejection leaves the lazy component without a rejected state or an error delivery path. Retain the rejection state, call triggerRender(), and throw the stored error during the next render.

🧰 Tools
🪛 Biome (2.5.5)

[error] 27-27: Expected an expression but instead found '.'.

(parse)

[error] 27-27: expected } but instead the file ends

(parse)

🪛 GitHub Actions: CI / 0_build-and-test.txt

[error] 27-27: tsup/esbuild build failed: Unexpected "." at the .catch(err => console.error(err)) expression. The @termuijs/jsx#build command exited with code 1.

🪛 GitHub Actions: CI / build-and-test

[error] 27-27: tsup/esbuild build failed: Unexpected "." at the .catch(err => console.error(err)) expression. The @termuijs/jsx build command 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/jsx/src/lazy.ts` at line 27, Update the lazy loader rejection
handling in the promise chain around the catch callback so it stores the
rejection as the lazy component’s rejected state, calls triggerRender(), and
delivers the stored loader error by throwing it during the next render; do not
handle the rejection with console.error alone.

Loading