Skip to content

fix: code quality and safety improvements #3377

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 #3377

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
2 changes: 1 addition & 1 deletion packages/cli/src/commands/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export async function runAdd(args: CliArgs): Promise<void> {
}
}

const deps = [...allDeps].sort();
const deps = [...allDeps].sort((a, b) => a - b);
if (deps.length === 0) return;

const pm = detectPackageManager();
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export async function resolveComponent(slug: string): Promise<ResolvedComponent>
...(json.deps ?? []),
...(json.peerDeps ?? []),
]),
].sort();
].sort((a, b) => a - b);

@coderabbitai coderabbitai Bot Aug 2, 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 | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

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

if rg -n 'sort\(\(a, b\) => a - b\)' \
  packages/cli/src/commands/add.ts \
  packages/cli/src/registry.ts \
  packages/tss/src/tokens.ts
then
  echo "Invalid numeric comparator remains" >&2
  exit 1
fi

npx tsc --noEmit --pretty false

Repository: Karanjot786/TermUI

Length of output: 437

🏁 Script executed:

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

echo "== file snippets =="
sed -n '60,76p' packages/cli/src/registry.ts | cat -n
printf '\n'
sed -n '76,90p' packages/cli/src/commands/add.ts | cat -n
printf '\n'
sed -n '95,112p' packages/tss/src/tokens.ts | cat -n

printf '\n== TypeScript config strictness indicators ==\n'
for f in tsconfig.json packages/*/tsconfig*.json; do
  [ -f "$f" ] || continue
  printf '%s\n' "$f"
  python3 - <<'PY' "$f"
import json, sys
p=sys.argv[1]
try:
    data=json.load(open(p))
except Exception as e:
    print("JSON/read error:", e); raise SystemExit
print(json.dumps(data.get("compilerOptions", {}), indent=2)[:800].replace("\n"," "))
PY
done

printf '\n== runtime numeric-subtraction behavior for representative strings =="
node - <<'JS'
const samples = [['a','b'], ['a', '42'], ['10', '2'], undefined, null, {}, []]
for (const [a, b] of samples) console.log(`${JSON.stringify(a)} - ${JSON.stringify(b)} = ${a - b}`)
JS

printf '\n== type assignment check if tsc runs without repo code ==\n'
npm_exec="$(cd "$(dirname "$(command -v npm)")" && pwd -P)/$(basename "$(command -v npm)")"
"$npm_exec" exec --package typescript -- tsc --noEmit --strict --declaration --skipLibCheck --allowJs false <<'TS'
type T = string[];
const deps = ['a', '10'];
deps.sort((a, b) => a - b);
TS

Repository: Karanjot786/TermUI

Length of output: 5048

🏁 Script executed:

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

echo "== TSConfig inheritance references =="
for f in tsconfig.json packages/cli/tsconfig.json packages/tss/tsconfig.json packages/tsconfig.*.json; do
  [ -f "$f" ] || continue
  printf '%s\n' "$f"
  node -e "const fs=require('node:fs'); const j=JSON.parse(fs.readFileSync(process.argv[1],'utf8')); console.log(JSON.stringify(j, null, 2))" "$f"
done

printf '\n== TypeScript numeric-subtraction assignment check (standalone) ==\n'
npm exec --package typescript -- tsc --noEmit --strict --declaration --skipLibCheck --allowJs false <<'TS'
type T = string[];
const deps: T = ['a', '10'];
deps.sort((a, b) => a - b);
TS

printf '\n== Runtime numeric-subtraction behavior for representative strings ==\n'
node - <<'JS'
const samples = [['a','b'], ['a', '42'], ['10', '2'], undefined, null, {}, []]
for (const [a, b] of samples) console.log(`${JSON.stringify(a)} - ${JSON.stringify(b)} = ${a - b}`)
JS

Repository: Karanjot786/TermUI

Length of output: 6329

🏁 Script executed:

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

npm_exec="$(node -e "const path=require('node:path'); const which=process.env.npm_execpath || path.join(process.cwd(), process.env.npm_config_user_prefix || '.', 'bin/npm'); console.log(which)")"
"$npm_exec" exec --package typescript -- tsc --noEmit --strict --declaration --skipLibCheck --allowJs false <<'TS'
type T = string[];
const deps: T = ['a', '10'];
deps.sort((a, b) => a - b);
TS

node - <<'JS'
const samples = [['a','b'], ['a', '42'], ['10', '2'], undefined, null, {}, []]
for (const [a, b] of samples) console.log(`${JSON.stringify(a)} - ${JSON.stringify(b)} = ${a - b}`)
JS

Repository: Karanjot786/TermUI

Length of output: 228

🏁 Script executed:

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

tmp="$(mktemp -d)"
cd "$tmp"

cat > tsconfig.json <<'JS'
{
  "compilerOptions": {
    "target": "ES2022",
    "module": "ESNext",
    "strict": true,
    "skipLibCheck": true
  }
}
JS

cat > test.ts <<'TS'
type T = string[];
const deps: T = ['a', '10'];
deps.sort((a, b) => a - b);
TS

npm exec --package typescript -- tsc --noEmit test.ts || true

node - <<'JS'
const samples = [['a','b'], ['a', '42'], ['10', '2'], undefined, null, {}, []]
for (const [a, b] of samples) console.log(`${JSON.stringify(a)} - ${JSON.stringify(b)} = ${a - b}`)
JS

Repository: Karanjot786/TermUI

Length of output: 1325

🏁 Script executed:

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

tmp="$(mktemp -d)"
cd "$tmp"

cat > tsconfig.json <<'JS'
{
  "compilerOptions": {
    "target": "ES2022",
    "module": "ESNext",
    "strict": false,
    "skipLibCheck": true
  }
}
JS

cat > test.ts <<'TS'
type T = string[];
const deps: T = ['a', '10'];
deps.sort((a, b) => a - b);
TS

node - <<'JS'
const samples = [['a','b'], ['a', '42'], ['10', '2'], undefined, null, {}, []]
for (const [a, b] of samples) console.log(`${JSON.stringify(a)} - ${JSON.stringify(b)} = ${String(a - b)}`)
JS

node - <<'TS'
const files = ['test.ts']
const ts = await import('typescript')
const host = ts.createCompilerHost({
  ...ts.getDefaultCompilerOptions(),
  strict: false,
  skipLibCheck: true,
})
host.readFile = (file) => file === '/tmp/test.ts' ? 'type T = string[];\nconst deps: T = [\'a\', \'10\'];\ndeps.sort((a, b) => a - b);\n' : undefined
const prog = ts.createProgram(files, ts.getDefaultCompilerOptions(), host)
const diags = ts.getPreEmitDiagnostics(prog)
console.log(diags.map(d => `${d.file ? d.file.fileName : ''}${d.start ?? ''}:${d.length ?? ''} TS${d.code} ${d.messageText}`).join('\n'))
TS

Repository: Karanjot786/TermUI

Length of output: 879

Replace numeric comparators on string identifiers.

a - b should not sort dependency names, allDeps, or token keys. These values are strings, so the expression coerces unsupported strings to NaN; use deterministic string sorting instead.

📍 Affects 3 files
  • packages/cli/src/registry.ts#L70-L70 (this comment)
  • packages/cli/src/commands/add.ts#L83-L83
  • packages/tss/src/tokens.ts#L105-L105
🤖 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/cli/src/registry.ts` at line 70, Replace the numeric a - b
comparators used to sort string identifiers with deterministic lexicographic
string sorting. Apply this to packages/cli/src/registry.ts lines 70-70,
packages/cli/src/commands/add.ts lines 83-83, and packages/tss/src/tokens.ts
lines 105-105, covering dependency names, allDeps, and token keys without
changing the surrounding sorting behavior.

Source: Coding guidelines

return {
name: json.name ?? slug,
slug,
Expand Down
2 changes: 1 addition & 1 deletion packages/jsx/src/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ export function useInterval(callback: () => void, delayMs: number): void {
* useMemo — memoize expensive computations.
*
* ```tsx
* const sorted = useMemo(() => items.sort(), [items]);
* const sorted = useMemo(() => items.sort((a, b) => a - b), [items]);
* ```
*/
export function useMemo<T>(factory: () => T, deps: any[]): T {
Expand Down
2 changes: 1 addition & 1 deletion packages/tss/src/tokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export function compileTokensToJSON(source: string, theme = 'default'): Compiled
}

const tokens: Record<string, string> = {};
for (const key of Object.keys(merged).sort()) {
for (const key of Object.keys(merged).sort((a, b) => a - b)) {
tokens[key] = resolveTokenValue(key, merged, new Set<string>());
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ui/src/MultiSelect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class MultiSelect extends Widget {
}

get selectedOptions(): MultiSelectOption[] {
return [...this._checked].sort().map(i => this._options[i]);
return [...this._checked].sort((a, b) => a - b).map(i => this._options[i]);
}
selectNext(): void { if (this._options.length === 0) return; let n = this._cursorIndex + 1; while (n < this._options.length && this._options[n].disabled) n++; if (n < this._options.length) { this._cursorIndex = n; this.markDirty(); } }
selectPrev(): void { if (this._options.length === 0) return; let n = this._cursorIndex - 1; while (n >= 0 && this._options[n].disabled) n--; if (n >= 0) { this._cursorIndex = n; this.markDirty(); } }
Expand Down
Loading