Skip to content

refactor: build js/util code from src/util code#87

Merged
Jason2866 merged 6 commits intomainfrom
gen_js_util
Apr 7, 2026
Merged

refactor: build js/util code from src/util code#87
Jason2866 merged 6 commits intomainfrom
gen_js_util

Conversation

@Jason2866
Copy link
Copy Markdown
Owner

@Jason2866 Jason2866 commented Apr 7, 2026

Summary by CodeRabbit

  • New Features

    • Added command history functionality: navigate previous commands using Up/Down arrow keys with automatic restoration of unsent drafts
    • Commands are deduplicated and history is limited to 100 recent entries
  • Style

    • Updated console styling with refined visual presentation

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

Warning

Rate limit exceeded

@Jason2866 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 28 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 28 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9387f8af-1539-4b8d-be83-1dae3129eb62

📥 Commits

Reviewing files that changed from the base of the PR and between 5e084a9 and 6e48f84.

📒 Files selected for processing (3)
  • js/console.js
  • package.json
  • tsconfig.util.json
📝 Walkthrough

Walkthrough

The pull request adds command-history functionality to ESP32ToolConsole, enabling users to navigate prior commands via arrow keys while maintaining draft input. Supporting build configuration updates introduce TypeScript compilation for utility modules, and multiple utility files are reformatted for consistency.

Changes

Cohort / File(s) Summary
Command History Feature
src/console.ts, js/console.js
Added command history state (commandHistory, historyIndex, currentInput) with _navigateHistory() method to navigate prior commands via ArrowUp/ArrowDown keys. History entries are deduplicated and capped at 100; navigation state resets when other keys are pressed or commands are sent.
Build Configuration
package.json, tsconfig.util.json
Added TypeScript compilation step (tsc -p tsconfig.util.json) to build scripts and dev watch mode script; new config file targets src/util/*.ts and outputs to js/util with strict type-checking enabled.
Code Formatting & Refactoring
js/util/console-color.js, js/util/line-break-transformer.js, js/util/timestamp-transformer.js
Re-indented and reformatted code structure (braces, line breaks, CSS rule blocks) without changing executable logic. In console-color.js, adjusted .log border-radius from 0 to 3px and removed min-height; in timestamp-transformer.js, updated comment reference from console-color.js to console-color.ts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #86: Implements identical command-history feature in ESP32ToolConsole with the same state management and navigation mechanics.

Poem

🐰 A history of hops, now saved in time,
Up and down arrows, each command sublime,
No more forgotten words from yesterday's quest,
The console remembers—what coding achievement!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main refactor: building JavaScript utility code (js/util) from TypeScript source (src/util), which is reflected in the new tsconfig.util.json, updated build scripts, and converted source files.
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
  • Commit unit tests in branch gen_js_util

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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 and usage tips.

@Jason2866
Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

✅ Actions performed

Full review triggered.

@Jason2866
Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

✅ Actions performed

Full review triggered.

@Jason2866
Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
tsconfig.util.json (1)

13-13: Consider if recursive glob is needed.

The pattern src/util/*.ts only matches files directly in src/util/, not nested subdirectories. If utility files may be organized into subfolders in the future, consider using src/util/**/*.ts instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tsconfig.util.json` at line 13, The tsconfig include uses a non-recursive
glob "src/util/*.ts" which only matches files directly under src/util; if
utilities may live in subfolders, update the include pattern to a recursive glob
such as "src/util/**/*.ts" so nested .ts files are picked up by the compiler and
tooling (replace the existing "src/util/*.ts" entry in tsconfig.util.json).
js/console.js (1)

1-418: Consider documenting the dual-maintenance requirement.

js/console.js and src/console.ts implement identical command-history logic but are maintained independently (different build outputs, different feature sets like ImprovDialog). Future bug fixes or enhancements to the history feature must be applied to both files.

Consider adding a code comment near the history fields referencing the other file to help future maintainers keep them synchronized.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@js/console.js` around lines 1 - 418, Add a short code comment above the
command-history fields (commandHistory, historyIndex, currentInput) in
ESP32ToolConsole noting that the history implementation is duplicated in
src/console.ts and must be updated in both places; include the other file name
and a brief note like "keep in sync with src/console.ts" so future maintainers
see the dual-maintenance requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@js/console.js`:
- Around line 1-418: Add a short code comment above the command-history fields
(commandHistory, historyIndex, currentInput) in ESP32ToolConsole noting that the
history implementation is duplicated in src/console.ts and must be updated in
both places; include the other file name and a brief note like "keep in sync
with src/console.ts" so future maintainers see the dual-maintenance requirement.

In `@tsconfig.util.json`:
- Line 13: The tsconfig include uses a non-recursive glob "src/util/*.ts" which
only matches files directly under src/util; if utilities may live in subfolders,
update the include pattern to a recursive glob such as "src/util/**/*.ts" so
nested .ts files are picked up by the compiler and tooling (replace the existing
"src/util/*.ts" entry in tsconfig.util.json).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b47947c-2f02-4d54-bc30-94d5eb3843cb

📥 Commits

Reviewing files that changed from the base of the PR and between 74bc432 and 5e084a9.

⛔ Files ignored due to path filters (11)
  • apple-touch-icon.png is excluded by !**/*.png
  • icons/icon-128.png is excluded by !**/*.png
  • icons/icon-144.png is excluded by !**/*.png
  • icons/icon-152.png is excluded by !**/*.png
  • icons/icon-192.png is excluded by !**/*.png
  • icons/icon-384.png is excluded by !**/*.png
  • icons/icon-512.png is excluded by !**/*.png
  • icons/icon-72.png is excluded by !**/*.png
  • icons/icon-96.png is excluded by !**/*.png
  • screenshots/desktop.png is excluded by !**/*.png
  • screenshots/mobile.png is excluded by !**/*.png
📒 Files selected for processing (7)
  • js/console.js
  • js/util/console-color.js
  • js/util/line-break-transformer.js
  • js/util/timestamp-transformer.js
  • package.json
  • src/console.ts
  • tsconfig.util.json

@Jason2866
Copy link
Copy Markdown
Owner Author

@coderabbitai the changes in package.json and tsconfig.util.json are the interesting in this PR. Does build the js/util from the source code src/util without issues?

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me inspect the relevant files to give you a thorough answer.


🏁 Script executed:

#!/bin/bash
echo "=== package.json ==="
cat package.json

echo ""
echo "=== tsconfig.util.json ==="
cat tsconfig.util.json

echo ""
echo "=== tsconfig.json (base) ==="
cat tsconfig.json 2>/dev/null || echo "NOT FOUND"

echo ""
echo "=== src/util directory ==="
ls -la src/util/ 2>/dev/null || echo "NOT FOUND"

echo ""
echo "=== src/util files ==="
for f in src/util/*.ts; do
  echo "--- $f ---"
  cat "$f"
  echo ""
done

echo ""
echo "=== js/util directory ==="
ls -la js/util/ 2>/dev/null || echo "NOT FOUND"

echo ""
echo "=== Diff of js/util vs src/util (structure check) ==="
diff <(ls src/util/ | sed 's/\.ts$//') <(ls js/util/ | sed 's/\.js$//') 2>/dev/null || echo "Could not diff"

Length of output: 17754


@Jason2866 The build configuration looks correct and should work without issues. Here's a detailed breakdown:


✅ What works correctly

  1. No double-compilation conflict: The base tsconfig.json uses "include": ["src/*"]. The single * glob does not cross directory boundaries in TypeScript, so src/util/*.ts files are excluded from the base compilation and only compiled by tsconfig.util.json. No conflicts.

  2. Library coverage: tsconfig.util.json includes "lib": ["es2019", "dom"], which covers all DOM types used by the util sources:

    • console-color.tsHTMLElement, Element, DocumentFragment, document.createElement
    • line-break-transformer.ts and timestamp-transformer.tsTransformer<string, string>, TransformStreamDefaultController
  3. Correct build order in package.json:

    tsc --skipLibCheck  →  tsc -p tsconfig.util.json  →  rollup -c
    

    Util files are compiled before rollup bundles, which is the right order.

  4. Output target matches: target: "es2019" and module: "es2020" are identical to the base tsconfig.json, ensuring consistent output across the project.


⚠️ Minor issues to be aware of

  1. js/util is not cleaned during prebuild: The prebuild script cleans dist and js/modules but not js/util. If any .ts file in src/util is ever renamed or deleted, the old compiled .js file will linger in js/util. Consider adding a cleanup step:

    "prebuild": "node -e \"const fs=require('fs'); fs.rmSync('dist',{recursive:true,force:true}); fs.rmSync('js/modules',{recursive:true,force:true}); fs.rmSync('js/util',{recursive:true,force:true}); fs.mkdirSync('js/modules',{recursive:true}); fs.mkdirSync('js/util',{recursive:true});\""
  2. tsconfig.util.json could extend the base config: To stay in sync with strictness options like noFallthroughCasesInSwitch, noImplicitReturns, noUnusedLocals, and forceConsistentCasingInFileNames, you could add:

    {
      "extends": "./tsconfig.json",
      "compilerOptions": {
        "outDir": "js/util",
        "declaration": false,
        "removeComments": false
      },
      "include": ["src/util/*.ts"]
    }

    This would reduce duplication and ensure the util build always inherits the same strictness as the main build.

@Jason2866 Jason2866 merged commit d1e9de2 into main Apr 7, 2026
3 checks passed
@Jason2866 Jason2866 deleted the gen_js_util branch April 7, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant