Fix Json Editor freezing on big input

[diegoimbert]

Important

Fix JSON editor freezing by limiting input size and adding a user option to load large inputs.

• Behavior:
  • Limits JSON input to 5000 characters in FakeMonacoPlaceHolder.svelte to prevent freezing.
  • Adds a button in JsonEditor.svelte to allow users to load large JSON inputs if they exceed 1,000,000 characters.
• Components:
  • Imports Button component in JsonEditor.svelte for user interaction.
• Variables:
  • Introduces CHAR_LIMIT in FakeMonacoPlaceHolder.svelte to cap input size.
  • Adds loadTooBigAnyway flag in JsonEditor.svelte to manage large input loading.

^{This description was created by for d4eaa1c. You can customize this summary. It will automatically update as commits are pushed.}

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	d4eaa1c
Status:	✅  Deploy successful!
Preview URL:	https://c862ed9e.windmill.pages.dev
Branch Preview URL:	https://di-fix-json-editor-big-input.windmill.pages.dev

View logs

[claude]

Claude finished @diegoimbert's task —— View job

---

PR Code Review - JsonEditor Performance Fix

Review Tasks:

• Read and analyze the JsonEditor.svelte changes
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance implications
• Assess security considerations
• Provide comprehensive feedback and suggestions

Changes Overview

The PR addresses JsonEditor freezing issues with large JSON inputs by:

1. Reducing threshold: tooBig from 1,000,000 to 100,000 characters
2. Preventing parsing: Early return in parseJson() when JSON is too big
3. UI changes: Always show SimpleEditor with placeholder text for large JSON
4. Removed conditional rendering: No longer shows "JSON to edit is too big" message

Code Review Feedback

✅ Strengths

• Performance improvement: The 100,000 character threshold is much more reasonable and will prevent UI freezes
• Early return logic: Preventing JSON parsing for large inputs is a good optimization
• Maintains component interface: All existing props and events are preserved
• Error handling: JSON parsing is still properly wrapped in try-catch

⚠️ Issues & Concerns

1. Complex bind:code Implementation (JsonEditor.svelte:49)

bind:code={() => (tooBig ? '// JSON is too big to edit' : code), (c) => (code = c)}

• This getter/setter function approach is unusual and potentially confusing
• The arrow function syntax makes it hard to read
• Consider extracting this logic into separate functions for clarity

2. User Experience Issues

• Unclear feedback: Users see "// JSON is too big to edit" but may not understand why they can't edit
• Missing visual indicator: No clear visual distinction that this is a disabled state
• Lost context: The original "JSON to edit is too big" message was more explicit

3. Potential State Management Issues

• When tooBig changes from true to false, the editor might not properly sync with the actual code
• The SimpleEditor might maintain internal state that conflicts with the placeholder text

4. Edge Case Handling

• What happens if user tries to type in the editor when tooBig is true?
• The setter (c) => (code = c) will still update the code variable, potentially causing inconsistencies

📋 Suggestions for Improvement

1. Improve bind:code readability

bind:code={getDisplayCode, setCode}

function getDisplayCode() {
  return tooBig ? '// JSON is too big to edit' : code;
}

function setCode(newCode) {
  if (!tooBig) {
    code = newCode;
  }
}

2. Better user feedback

{#if tooBig}
  <div class="bg-yellow-50 border border-yellow-200 text-yellow-800 px-3 py-2 text-sm">
    ⚠️ JSON is too large to edit (>{100000.toLocaleString()} characters). View-only mode.
  </div>
{/if}

3. Consider making editor truly readonly

<SimpleEditor
  readonly={tooBig}
  {/* other props */}
/>

4. Add loading state for large JSON

{#if tooBig}
  <div class="text-gray-500 italic text-sm">
    Large JSON detected. Parsing disabled for performance.
  </div>
{/if}

🔧 Performance Considerations

• ✅ Good: 100,000 character threshold prevents parsing bottlenecks
• ✅ Good: Early return in parseJson() avoids expensive operations
• ⚠️ Consider: SimpleEditor still renders for large JSON - might want to use a simpler text display for very large inputs

🔒 Security Assessment

• ✅ No security issues identified
• ✅ JSON parsing still properly wrapped in try-catch
• ✅ No risk of code injection through the placeholder text

📊 Testing Recommendations

• Test with JSON exactly at 100,000 characters
• Test toggling between large and small JSON
• Test user interaction when tooBig is true
• Verify editor state consistency after size changes

Summary

The core fix is solid and addresses the performance issue effectively. The main concerns are around UX clarity and the complexity of the bind:code implementation. Consider the suggested improvements to make the behavior more explicit and user-friendly.

---

[graphite-app]

The bind:code syntax here is incorrect - Svelte's binding directives don't accept functions as arguments. This will cause runtime errors or unexpected behavior.

For handling large JSON, consider one of these approaches:

1. Keep the standard binding and handle the size check in a reactive statement:

bind:code={code}

$: if (tooBig && code && !code.startsWith('// JSON is too big')) {
  code = '// JSON is too big to edit';
}

2. Or restore the conditional rendering approach that was removed:

{#if tooBig}
  <SimpleEditor code="// JSON is too big to edit" readonly />
{:else}
  <SimpleEditor bind:code={code} />
{/if}

Either approach would maintain the intended functionality while using valid Svelte syntax.

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 132b9cc in 1 minute and 33 seconds. Click for details.

• Reviewed 64 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/lib/components/JsonEditor.svelte:15

• Draft comment:
  Threshold lowered to 100K (from 1M). Confirm this change doesn’t inadvertently block valid JSON inputs.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_LxalGWOLb0C35KIB

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Using a comma operator in bind:code causes only the setter to be passed, discarding the custom getter. This likely doesn’t provide the intended '// JSON is too big to edit' display.

Suggested change

	bind:code={() => (tooBig ? '// JSON is too big to edit' : code), (c) => (code = c)}
	bind:code={{ get: () => (tooBig ? '// JSON is too big to edit' : code), set: (c) => (code = c) }}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 1764041 in 43 seconds. Click for details.

• Reviewed 31 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/lib/components/FakeMonacoPlaceHolder.svelte:39

• Draft comment:
  Limiting the preview to the first 5000 characters improves performance on large inputs. Consider documenting the reason for this magic number or making it configurable for clarity.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. frontend/src/lib/components/JsonEditor.svelte:15

• Draft comment:
  The tooBig threshold is increased to 1,000,000 characters, which allows larger JSON inputs to be parsed. Confirm that this change doesn't reintroduce performance issues and consider adding a comment on the rationale behind this threshold increase.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_4y6WACn74sLif75Y

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed d4eaa1c in 1 minute and 22 seconds. Click for details.

• Reviewed 76 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/lib/components/JsonEditor.svelte:22

• Draft comment:
  Removal of the guard (if (tooBig) return) in parseJson may lead to expensive JSON.parse calls on huge inputs. Consider conditionally skipping parsing unless the user has clicked 'Load anyway'.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. frontend/src/lib/components/JsonEditor.svelte:36

• Draft comment:
  The reactive statement ($: code != undefined && parseJson()) always triggers JSON.parse even for huge inputs. Consider adding a condition (e.g., !tooBig || loadTooBigAnyway) to avoid unnecessary parsing before the user opts to load.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. frontend/src/lib/components/JsonEditor.svelte:16

• Draft comment:
  The magic number 1000000 used to define 'tooBig' could be extracted into a named constant to improve maintainability and readability.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. frontend/src/lib/components/JsonEditor.svelte:36

• Draft comment:
  If the code input is updated rapidly, repeatedly calling parseJson() might hurt performance. Consider debouncing the parseJson call.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_C45IeayJIC7EsDCV

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}