Skip to content

Conversation

@jfox-box
Copy link
Contributor

@jfox-box jfox-box commented Jan 22, 2026

This is a fix for this user-reported issue: #4335

In Vite apps dev mode, opening a pdf would result in an infinite loading spinner. This was due to the cloneDeep method in ContentPreview attempting to assign readOnly properties in a PDFViewer object. We determined that it would be okay to pass the data object without cloning it, since consumers are unlikely to run into problems if they mutate this object, and it actually provides additional functionality if they want to call methods on the PDFViewer object.

@jfox-box jfox-box requested a review from a team as a code owner January 22, 2026 21:18
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Walkthrough

Removed the lodash/cloneDeep dependency from the PreviewDialog component and modified the onLoad handler to pass data directly to the onPreview callback instead of passing a deep-cloned copy.

Changes

Cohort / File(s) Summary
Dependency cleanup
src/elements/common/preview-dialog/PreviewDialog.tsx
Removed cloneDeep import and updated onLoad handler to pass data directly without deep cloning, reducing external dependency usage

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

ready-to-merge

Suggested reviewers

  • greg-in-a-box
  • jpan-box

Poem

🐰 A clone's gone, the data flows free,
Lodash begone, one less import to see,
Direct and simple, efficient and clean,
The lightest refactor we've ever seen! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title mentions 'pdf infinite spinning loader issue' but the actual change removes cloneDeep from an onPreview call in PreviewDialog—the title does not accurately reflect the implemented solution. Update the title to reflect the actual change, such as: 'fix(content-explorer): Remove cloneDeep from onPreview call' to match the PR's actual implementation.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description adequately explains the issue being fixed, references the related GitHub issue, and provides clear rationale for the change.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

@jfox-box jfox-box changed the title fix(content-explorer): Remove cloneDeep from onPreview call fix(content-explorer): Fix pdf infinite spinning loader issue Jan 22, 2026
@mergify mergify bot added the queued label Jan 22, 2026
@mergify mergify bot merged commit 7d428f4 into master Jan 22, 2026
14 checks passed
@mergify mergify bot deleted the onPreview-cloneDeep branch January 22, 2026 22:35
@mergify
Copy link
Contributor

mergify bot commented Jan 22, 2026

Merge Queue Status

✅ The pull request has been merged at d015724

This pull request spent 5 seconds in the queue, with no time running CI.
The checks were run in-place.

Required conditions to merge

@mergify mergify bot removed the queued label Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants