-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Description
THINGS TO KEEP IN MIND BEFORE YOU BEGIN
-
Please put some thought into what console messages you remove. Most are redundant or left in place while debugging a new feature. But some are useful for on-going maintenance and support. For example, it is useful to be able see what notes have been played by the synth.
-
Speaking of maintenance, please consolidate your changes into a single PR per file. Otherwise the burden on the reviewers is too much.
The codebase currently contains 418 console.log/console.debug statements scattered across 54 JavaScript files. These debug statements:
- 🔴 Leak internal state/data to browser console (visible to end users)
- 🔴 Create noise during actual debugging sessions
- 🔴 Can impact performance in tight loops
- 🔴 Look unprofessional in production
This issue tracks the systematic removal/replacement of these statements with proper logging practices.
Audit Summary
| File | Count | Priority |
|---|---|---|
js/blocks.js |
🔴 Critical | |
js/utils/musicutils.js |
35 | 🔴 High |
js/utils/synthutils.js |
31 | 🔴 High |
js/activity.js |
27 | 🔴 High |
js/utils/utils.js |
24 | 🔴 High |
js/logo.js |
15 | 🟠 Medium |
js/blocks/ProgramBlocks.js |
15 | 🟠 Medium |
js/lilypond.js |
14 | 🟠 Medium |
js/js-export/generate.js |
12 | 🟠 Medium |
js/blocks/EnsembleBlocks.js |
11 | 🟠 Medium |
js/palette.js |
11 | 🟠 Medium |
| Other 43 files | 137 | 🟡 Lower |
| TOTAL | 418 | 54 files |
How to Contribute (Read Carefully)
We welcome contributions! Here's how to help:
Step 1: Pick a file
Choose an unchecked file from the checklist below.
Step 2: Remove console statements
- Follow the Guidelines section below
- Test that Music Blocks still works
- One file per PR (keeps changes small and reviewable)
Step 3: Format your code (⚠️ Required)
To avoid failing the automated checks, you MUST run Prettier on the specific file you edited before committing.
Run this command in your terminal:
npx prettier --write js/path/to/your/file.jsStep 4: Submit PR
- Title:
chore: remove debug console statements from <filename> - Description: Add
Part of #5464(this issue number) at the top of PR description - Run tests to ensure nothing breaks
Step 5: Get it merged!
Small, focused PRs are reviewed fastest. 🎉
Pro tip: Start with 🟡 Lower Priority files (1-7 statements) for easier first contributions.
Note: No need to ask permission first - just submit focused PRs following the guidelines. If you're unsure about a specific console statement, mention it in the PR description.
⏱️ Pacing (Important!)
There are many open PRs in the repo. To avoid overwhelming maintainers:
- Wait for your PR to be merged before submitting another
- 1 file = 1 PR — don't bundle multiple files
- This is a gradual cleanup, not a race
Quality over speed. Let's keep the review queue manageable! 🙏
⚠️ IMPORTANT: What to Keep vs Remove
DO NOT delete these (important logs):
- ❌
console.error()— Error handling, keep these - ❌
console.warn()— Warnings to developers/users - ❌ Logs inside
catchblocks — Exception diagnostics - ❌ Logs with error messages like
"Failed to...","Error:...","Invalid..."
SAFE to remove (debug spam):
- ✅
console.log(variableName)— Pure variable dumps - ✅
console.debug(...)— Debug-only output - ✅ Logs like
"here","test","xxx","checking..." - ✅ Logs that print arrays/objects for inspection
Quick Reference Table
| Situation | Action |
|---|---|
Pure debug output (console.log(x)) |
Remove entirely |
| Error logging in catch block | 🛑 KEEP or convert to console.error() |
| Useful diagnostic info | Wrap in if (this.debugMode) or similar |
| User-facing warnings | 🛑 KEEP console.warn() if intentional |
When in Doubt
If you're unsure whether a console statement is important:
- Keep it and mention it in your PR description
- A maintainer will review and advise
Files Checklist
🔴 Critical Priority - Already Claimed
The following 5 files are being handled and PRs are in progress:
- ✅
js/blocks.js(86 statements) - PR Submitted - Removes 81 statements - 🚧
js/utils/musicutils.js(35 statements) - In Progress - 🚧
js/utils/synthutils.js(31 statements) - In Progress - 🚧
js/activity.js(27 statements) - In Progress - 🚧
js/utils/utils.js(24 statements) - In Progress
Contributors: Please start with 🟠 Medium or 🟡 Lower priority files below.
🟠 Medium Priority (78 statements)
- ✅
js/logo.js(15 statements) - PR Chore remove debug console logo.js #5573 -
js/blocks/ProgramBlocks.js(15 statements) - Program blocks -
js/lilypond.js(14 statements) - LilyPond export -
js/js-export/generate.js(12 statements) - JS code generation -
js/blocks/EnsembleBlocks.js(11 statements) - Ensemble blocks -
js/palette.js(11 statements) - Palette operations
🟡 Lower Priority - Widgets (35 statements)
-
js/widgets/phrasemaker.js(8 statements) -
js/widgets/help.js(7 statements) -
js/widgets/modewidget.js(7 statements) -
js/widgets/reflection.js(4 statements) - ✅
js/widgets/status.js(4 statements) - PR chore: remove debug console statements from js/widgets/status.js #5476 - ✅
js/widgets/sampler.js(2 statements) - PR chore: remove debug console statements from js/widgets/sampler.js #5470 -
js/widgets/rhythmruler.js(2 statements) - ✅
js/widgets/musickeyboard.js(2 statements) - PR chore: remove debug console statements from js/widgets/musickeyboard.js #5479 -
js/widgets/pitchdrummatrix.js(1 statement) - ✅
js/widgets/tempo.js(1 statement) - PR chore:remove debug console statement from tempo widget #5469 - ✅
js/widgets/aidebugger.js(1 statement) - PR chore: removed debug console log and replaced deprecated substr #5534 - ✅
js/widgets/meterwidget.js(1 statement) - PR chore: remove console debug statement from meter widget #5471 - ✅
js/widgets/jseditor.js(1 statement) - PR chore: remove debug console statements from jseditor widget #5567 -
js/widgets/legobricks.js(1 statement)
🟡 Lower Priority - Blocks (43 statements)
-
js/blocks/BooleanBlocks.js(7 statements) -
js/block.js(5 statements) -
js/blocks/NumberBlocks.js(3 statements) -
js/blocks/ActionBlocks.js(2 statements) -
js/blocks/IntervalsBlocks.js(2 statements) -
js/blocks/PitchBlocks.js(2 statements) -
js/blocks/DrumBlocks.js(1 statement) - ✅
js/blocks/OrnamentBlocks.js(1 statement) - PR chore: remove debug console statement from OrnamentBlocks.js #5555
🟡 Lower Priority - Other Core (62 statements)
-
js/p5-sound-adapter.js(7 statements) -
js/turtles.js(7 statements) -
js/midi.js(7 statements) -
js/piemenus.js(6 statements) -
js/toolbar.js(6 statements) -
js/SaveInterface.js(4 statements) -
js/js-export/ast2blocklist.js(4 statements) -
js/pastebox.js(2 statements) -
js/themebox.js(2 statements) -
js/rubrics.js(2 statements) -
js/p5-adapter.js(2 statements) -
js/turtle-singer.js(1 statement) -
js/abc.js(1 statement) -
js/turtleactions/DictActions.js(1 statement) -
js/turtle.js(1 statement)
⚪ Test Files (Exclude from cleanup - debug needed)
js/js-export/__tests__/generate.test.js(7 statements)js/blocks/__tests__/DrumBlocks.test.js(3 statements)js/__tests__/SaveInterface.test.js(3 statements)js/turtleactions/__tests__/DictActions.test.js(2 statements)js/utils/__tests__/musicutils.test.js(2 statements)js/js-export/__tests__/ast2blocklist.test.js(2 statements)
Final Step: Add ESLint Rule
After all files are cleaned up:
- Add
no-consoleESLint rule to prevent future additions - Configure exceptions for intentional
console.error()andconsole.warn() - Update CI to enforce the rule
Proposed .eslintrc addition:
{
"rules": {
"no-console": ["error", { "allow": ["warn", "error"] }]
}
}Progress Tracker
| Phase | Files | Statements | Status | PRs Merged |
|---|---|---|---|---|
| 🔴 Critical Priority | 5 | 203 | 🟡 In Progress | 0/5 (1 pending) |
| 🟠 Medium Priority | 6 | 78 | 🟡 In Progress | 0/6 (1 pending) |
| 🟡 Widgets | 14 | 35 | 🟡 In Progress | 1/14 (6 pending) |
| 🟡 Block Files | 8 | 23 | 🟡 In Progress | 1/8 |
| 🟡 Other Core | 15 | 60 | ⬜ Not Started | 0/15 |
| ESLint Rule | 1 | - | ⬜ Not Started | 0/1 |
| TOTAL | 49 | 399* | 16% | 2/49 (9 pending) |
*Excludes 19 statements in test files which are acceptable
PRs
Pending Review
| PR | File | Statements Removed | Contributor | Date |
|---|---|---|---|---|
| #5465 | js/blocks.js |
81 | @Inuth0603 | 2026-02-01 |
| #5469 | js/widgets/tempo.js |
1 | @Ayush78516 | 2026-02-02 |
| #5470 | js/widgets/sampler.js |
2 | @AnikeshAnilkumarLathe | 2026-02-02 |
| #5471 | js/widgets/meterwidget.js |
1 | @Shruti0460 | 2026-02-02 |
| #5476 | js/widgets/status.js |
4 | @thevanshit | 2026-02-02 |
| #5479 | js/widgets/musickeyboard.js |
2 | @abhiraj75 | 2026-02-02 |
| #5567 | js/widgets/jseditor.js |
1 | @abhnish | 2026-02-06 |
| #5573 | js/logo.js |
2 | @bhangalesoham2606 | 2026-02-07 |
Merged
| PR | File | Statements Removed | Contributor | Date |
|---|---|---|---|---|
| #5555 | js/blocks/OrnamentBlocks.js |
1 | @niveshdandyan | 2026-02-06 |
| #5534 | js/widgets/aidebugger.js |
1 | @Oashe02 | 2026-02-06 |
Why This Matters
- Cleaner user experience - No debug spam in browser console
- Easier debugging - Less noise when actually debugging issues
- Professionalism - Production code shouldn't leak internals
- Performance - Fewer unnecessary function calls
- Maintainability - ESLint rule prevents regression
Related
- Part of ongoing code quality improvements
- Prepares codebase for v4 migration
Labels: chore, good first issue, help wanted, code quality
Milestone: v3.x Maintenance