fix(preview): add preview sdk load check

[bfoxx1906]

This PR fixes an edge case where if the file info for the preview is loaded but the preview sdk is not, then the ContentPreview gets stuck in a perpetual loading state. This can happen on clients with increased network latency. When the function shouldLoadPreview executes on componentDidUpdate it checks to see whether or not the file is loaded and if it's new relative to the preview. If it is then loadPreview is set to true and which leads ContentPreview to attempt to load the preview. If the preview sdk is not loaded then loadPreview will return without actually calling the preview sdk and showing the preview. This leads to the perpetual loading state because shouldLoadPreview, when it's called again in componentDidUpdate, will always return false because the file will no longer be considered new relative to the component's state.

To fix this, I added a new field to the ContentPreview which keeps track of the preview sdk loaded state. This is the basis of a new conditional in shouldLoadPreview PR Ref that will return true and ultimately cause the preview to load if the preview sdk wasn't previously loaded but currently is.

Before

After

Before Screenshot: The logpoint pointed to by the red arrow shows where loadPreview gets called but the preview sdk isn't loaded.

After Screenshot: The arrow on the top points to the logpoint message that gets logged in loadPreview if the preview is trying to load but the sdk isn't ready. The arrow on the bottom points to the logpoint messages that get logged when shouldLoadPreview returns true because the preview sdk wasn't ready before but is now and then when the preview sdk actually gets called in the loadPreview function.

Summary by CodeRabbit

• Bug Fixes
  • Prevents premature preview load attempts until the external preview library is available and triggers a one-time load when it becomes available.
• Tests
  • Added tests verifying preview load gating behavior and that no load attempts occur when prerequisites (library, file, or token) are missing.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a guard to ContentPreview that tracks external preview library availability via a new previewLibraryLoaded boolean, preventing preview loads until the library is present and ensuring a single load when the library becomes available after a file is set.

Changes

Cohort / File(s)	Summary
Preview library guard implementation src/elements/content-preview/ContentPreview.js	Adds previewLibraryLoaded: boolean (initialized false). Updates shouldLoadPreview to trigger when the library becomes available and a preview hasn't been loaded. Recomputes and synchronizes previewLibraryLoaded in loadPreview and related lifecycle flows to avoid premature or duplicate loads.
Test coverage for library guard src/elements/content-preview/__tests__/ContentPreview.test.js	Adds tests verifying shouldLoadPreview returns true when the library transitions to available and no preview loaded. Adds tests ensuring loadPreview does not call getFileId when library is unavailable, file is unset, or token is unset.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Review synchronization of previewLibraryLoaded updates across lifecycle methods to avoid race conditions.
• Verify shouldLoadPreview correctly reflects transitions from false→true and prevents repeated loads.
• Confirm tests cover the introduced guard and edge conditions (missing file/token).

Poem

🐰 I waited, watched the library bloom,
Now preview waits no more — it springs from its room.
One gentle load when the signal turns green,
No frantic retries, no glimpses unseen.
Hooray for tidy hops in the previewing team!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and clearly describes the main change: adding a preview SDK load check to fix the perpetual loading state issue.
Description check	✅ Passed	The description is comprehensive, explaining the bug, the root cause, the solution, and includes visual evidence with before/after screenshots.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22f5332 and 59cbe2a.

📒 Files selected for processing (1)

• src/elements/content-preview/__tests__/ContentPreview.test.js (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/elements/content-preview/tests/ContentPreview.test.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: lint_test_build
• GitHub Check: Summary

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 454ede0 and 22f5332.

📒 Files selected for processing (2)

• src/elements/content-preview/ContentPreview.js (3 hunks)
• src/elements/content-preview/__tests__/ContentPreview.test.js (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/elements/content-preview/__tests__/ContentPreview.test.js (1)

src/elements/content-preview/ContentPreview.js (3)

• props (291-301)
• props (322-322)
• props (377-377)

src/elements/content-preview/ContentPreview.js (6)

src/elements/content-preview/__tests__/ContentPreview.test.js (1)

• file (18-18)

src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js (1)

• file (74-79)

src/elements/content-sidebar/__tests__/ActivitySidebar.test.js (1)

• file (49-54)

src/api/__tests__/Feed.test.js (1)

• file (349-364)

src/elements/content-sidebar/__tests__/Sidebar.test.js (1)

• file (28-33)

src/api/File.js (1)

• file (229-229)

🪛 Biome (2.1.2)

src/elements/content-preview/ContentPreview.js

[error] 231-231: return types can only be used in TypeScript files

remove this type annotation

(parse)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Summary

🔇 Additional comments (6)

src/elements/content-preview/ContentPreview.js (3)

231-232: LGTM!

The new previewLibraryLoaded property correctly tracks the preview library's load state. The static analysis hint about TypeScript syntax is a false positive—this is valid Flow syntax (the file has @flow directive on line 2).

---

457-462: LGTM! Good fix for the race condition.

The guard correctly detects when the preview library becomes available after the file was already set, ensuring the preview load is retried. The !this.preview check appropriately prevents re-triggering if a preview instance already exists.

The side effect (setting previewLibraryLoaded = true) in a should* method is slightly unconventional, but the comment clearly explains the intent and it's a pragmatic solution for synchronizing state.

---

820-822: LGTM!

Updating previewLibraryLoaded before the guard ensures the instance property stays synchronized with the actual library state. This allows shouldLoadPreview to correctly detect when the library becomes available on subsequent updates if loadPreview returns early here.

src/elements/content-preview/__tests__/ContentPreview.test.js (3)

178-185: LGTM!

Good test coverage for the new guard logic. It correctly verifies both the return value and the side effect of setting previewLibraryLoaded to true.

---

505-514: LGTM!

Good test for the early return behavior. Using getFileId as a proxy to verify execution stopped early is a reasonable approach since it's called after the guard check.

---

525-533: LGTM!

Good test coverage for the token validation in the guard. Correctly verifies that loadPreview returns early when the token is not set.

[mergify]

Merge Queue Status

✅ The pull request has been merged at 59cbe2a

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

Required conditions to merge

• #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • fix(preview): add preview sdk load check #4391
• #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • fix(preview): add preview sdk load check #4391
• #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • fix(preview): add preview sdk load check #4391
• branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • fix(preview): add preview sdk load check #4391
• any of [🛡 GitHub branch protection]:
  • check-success = Summary
  • check-neutral = Summary
  • check-skipped = Summary
• any of [🛡 GitHub branch protection]:
  • check-success = lint_test_build
  • check-neutral = lint_test_build
  • check-skipped = lint_test_build
• any of [🛡 GitHub branch protection]:
  • check-success = license/cla
  • check-neutral = license/cla
  • check-skipped = license/cla
• any of [🛡 GitHub branch protection]:
  • check-success = lint_pull_request
  • check-neutral = lint_pull_request
  • check-skipped = lint_pull_request