-
Notifications
You must be signed in to change notification settings - Fork 35
Enhance Monaco Editor syntax error detection for save button #191
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.
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
Conversation
- Add formatOnType and formatOnPaste options for real-time code formatting - Implement format-on-save functionality in handleSave method - Gracefully handle format errors with fallback behavior 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…save button - Add Monaco Editor syntax error detection using marker API - Update LED indicator to show red when selected code has syntax errors - Modify save button to show error count and disable when errors exist - Add selectedCodeHasErrors state tracking from Monaco Editor to chat components - Implement real-time error monitoring with onDidChangeMarkers event listener - Connect syntax error state through ResultPreview → home.tsx → ChatInterface → MessageList → Message → StructuredMessage → CodeSegment - Save button now displays "X Error(s)" in red and is disabled when syntax errors present - LED indicator priority: orange (streaming) → red (errors) → green (selected) → gray (default) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Key fixes to make syntax error detection work: 1. **Language Service Fix**: Changed model language from `jsx` to `javascript` - JSX language is for highlighting only, JavaScript language service has error detection - Keeps JSX compilation enabled for proper JSX syntax validation 2. **Enable Diagnostics**: Added `setDiagnosticsOptions` with both validation types enabled - `noSemanticValidation: false` - enables semantic/type checking - `noSyntaxValidation: false` - enables syntax error detection 3. **Enhanced Compiler Options**: Added modern JSX support configuration - ESNext modules, Node.js resolution, esModuleInterop - Optimized for React JSX with proper factory settings 4. **Improved Error Detection**: Enhanced marker filtering and monitoring - Filter by `owner: 'typescript'` to get language service errors only - Optimized `onDidChangeMarkers` to only check relevant URI changes - Added debug logging to verify error detection (temporary) Now Monaco Editor will detect JSX syntax errors (unclosed tags, invalid expressions, etc.) and show them via the red LED indicator and disabled save button. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The issue was that handleCodeChange (called on every keystroke) was calling onSyntaxErrorChange(0) immediately, overriding the correct error count from the async onDidChangeMarkers listener. Key fixes: - Remove immediate syntax error checking from handleCodeChange - Rely solely on onDidChangeMarkers for accurate error detection - Add debug logging to trace error state changes - Monaco's TypeScript language service updates markers asynchronously This should fix the save button briefly showing red then reverting to blue. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
… tracking and debounced checks
✅ Deploy Preview for fireproof-ai-builder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update adds useful editor diagnostics but leaves heavy, unguarded console logging and un-debounced setTimeout calls that may degrade performance during active editing. A minor compatibility note regarding JSX highlighting is also flagged. No functional blockers were found, but cleaning up the debug artefacts and debouncing the error checks will improve runtime performance and maintainability.
Additional notes (2)
-
Compatibility |
app/components/ResultPreview/setupMonacoEditor.ts:86-87
Changing the model language fromjsxtojavascriptenables diagnostics but may slightly degrade JSX-specific syntax highlighting (e.g., TSX-style tokens). If you only need the JavaScript language service, another option is to keep the languagejsxand setmonaco.languages.typescript.javascriptDefaultsfor diagnostics, or override the theme. Just flagging in case the visual quality drops. -
Performance |
app/routes/home.tsx:128-132
Left-overconsole.logwill run on every syntax-error change and end up in production bundles unless stripped by the build. Same concern as above—remove or guard behindprocess.env.NODE_ENV !== 'production'.
Summary of changes
ResultPreview module enhancements
-
Error-aware save flow
- Propagated
syntaxErrorCountfrom the Monaco editor (IframeContent → ResultPreview → home.tsx) up to theSaveButton. SaveButtonnow shows a red disabled state with the error count and formats document before saving.
- Propagated
-
Monaco editor setup
- Switched model language to
javascript, enabled diagnostics, richer compiler options, and turned onformatOnType/formatOnPaste. - Added comprehensive marker tracking with listeners for
onDidChangeMarkersandonDidChangeModelContentincluding initial / delayed checks.
- Switched model language to
-
UI plumbing
- Added new optional props (
onSyntaxErrorChange,syntaxErrorCount) across several components. - Updated header content and routing to reflect the new save-button behaviour and state.
- Added new optional props (
-
Misc
- Numerous console debug logs for marker information and syntax-error state propagation.
| const contentDisposable = editor.onDidChangeModelContent(() => { | ||
| // Debounce content changes to avoid excessive checks | ||
| setTimeout(checkSyntaxErrors, 500); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setTimeout(checkSyntaxErrors, 500) is scheduled on every keystroke. Without debouncing or clearing previous timers this can queue hundreds of callbacks during rapid typing, causing unnecessary work.
Suggestion
Wrap the syntax-error check in a debounced function, e.g. using lodash.debounce, or manually clear the previous timeout before scheduling a new one.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CharlieHelps yes please - manual clear dont need lodash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expand this to see my work.
- Added
let syntaxErrorCheckTimeoutId: ReturnType<typeof setTimeout> | null = null;scoped to theonMountclosure inIframeContent.tsx. - Updated all previous
setTimeout(checkSyntaxErrors, …)calls (initial check, marker changes, editor content changes) to use a singlescheduleSyntaxCheckhelper that first callsclearTimeout(syntaxErrorCheckTimeoutId). - Confirmed there are no imports or references to
lodash.debounceor any external debounce utility inIframeContent.tsxafter a search. - Manually verified (in code around line 230–340) that the shared
syntaxErrorCheckTimeoutIdis now used at every relevant scheduling point and the old debounce comments now reference the new manual logic. - Validated the patch to
IframeContent.tsxwas applied cleanly and only lines documenting the new debouncing approach mention 'debounce' (no actual utility remains). - Ran
pnpm check(typecheck/lint/tests), saw only unrelated test noise, no errors forIframeContent.tsx. - Staged changes to
app/components/ResultPreview/IframeContent.tsxfor commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented manual timeout management for Monaco syntax-error checks.
- Added
syntaxErrorCheckTimeoutIdplusscheduleSyntaxCheck()helper inside theonMountblock
– clears any pending timer before queueing a new one
– shared across initial check,onDidChangeMarkers, andonDidChangeModelContenthandlers - Removed the unguarded
setTimeoutcalls; no lodash debounce needed
Commit: 65fc1c9
Let me know if you’d like any tweaks.
|
good work @CharlieHelps |
|
All done. |
|
Thanks for the feedback, @jchris—glad the update hit the mark. Just ping me if you’d like any further tweaks or improvements. |
Summary
Changes Made
Test Plan
🤖 Generated with Claude Code