Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions packages/docs/audits/local-first-roadmap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
title: Local-First Sync Roadmap (CoRATES)
date: 2026-01-07
author: Team
---

This roadmap turns the strategy from `docs/plans/local-first-sync-strategy.md` into a concrete, time-boxed plan with owners, milestones, and acceptance criteria. It is intentionally pragmatic: ship small, measure, and expand.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Correct file path reference.

Line 7 references docs/plans/local-first-sync-strategy.md, but the file is located at docs/audits/local-first-sync-strategy.md. Fix this broken cross-reference.

🔗 Proposed fix
- This roadmap turns the strategy from `docs/plans/local-first-sync-strategy.md` into a concrete, time-boxed plan with owners, milestones, and acceptance criteria. It is intentionally pragmatic: ship small, measure, and expand.
+ This roadmap turns the strategy from `docs/audits/local-first-sync-strategy.md` into a concrete, time-boxed plan with owners, milestones, and acceptance criteria. It is intentionally pragmatic: ship small, measure, and expand.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
This roadmap turns the strategy from `docs/plans/local-first-sync-strategy.md` into a concrete, time-boxed plan with owners, milestones, and acceptance criteria. It is intentionally pragmatic: ship small, measure, and expand.
This roadmap turns the strategy from `docs/audits/local-first-sync-strategy.md` into a concrete, time-boxed plan with owners, milestones, and acceptance criteria. It is intentionally pragmatic: ship small, measure, and expand.
🤖 Prompt for AI Agents
In @packages/docs/audits/local-first-roadmap.md at line 7, Update the incorrect
cross-reference string `docs/plans/local-first-sync-strategy.md` to the correct
path `docs/audits/local-first-sync-strategy.md` in the sentence that begins
"This roadmap turns the strategy..." so the link points to the actual file
location; ensure any other occurrences of
`docs/plans/local-first-sync-strategy.md` in this document are likewise replaced
with `docs/audits/local-first-sync-strategy.md`.


Timeline overview

- Immediate (0–2 weeks)
- Inventory endpoints and sync domains (who owns which endpoints).
- Add `GET /api/metadata` scaffold returning `{ apiVersion, minSupportedClient, cacheVersion, features }`.
- Add boot-time client check against `minSupportedClient` and non-blocking UI banner.
- Move a small set of canonical Zod schemas into `packages/shared` (start with user/session DTOs).

- Short (2–8 weeks)
- CI: generate TypeScript client from `openapi.json` and fail PRs if out-of-date.
- Implement `cacheVersion` propagation and persisted query invalidation on change.
- Implement persistent IndexedDB op-queue library (client) and `/api/sync/ops` endpoint (server).
- Add telemetry for client-version and op-queue backlog size.

- Medium (2–6 months)
- Move document sync to Yjs Durable Object relay (if not already), add migration plan for doc schema versions.
- Add server-side reconciliation endpoints and idempotent replay handling.
- Add integration tests for offline replay, Yjs sync, and forced-upgrade flows.

- Long (6+ months)
- Harden monitoring/alerting (alerts for high backlog, deprecated-client usage > X%).
- Full migration of remaining Zod schemas into `packages/shared` and strict CI gating.
- Optional: evaluate GraphQL for read-heavy aggregation endpoints (pilot only).

Owners & acceptance criteria

- Infrastructure: @backend-owner — scaffold `/api/metadata`, CI typegen, telemetry hooks.
- Client: @frontend-owner — boot-time version check, SW/BroadcastChannel update path, op-queue client library.
- Sync primitives: @sync-owner — Yjs relay, reconciliation endpoints, doc migrations.

Acceptance criteria (per milestone)

- Inventory & metadata scaffold
- Inventory file committed listing endpoints and whether they use Yjs/REST.
- `/api/metadata` responds with valid JSON and is documented in OpenAPI.
- Client shows non-blocking banner when `clientVersion < minSupportedClient`.

- CI typegen
- `openapi.json` updates produce regenerated client code via CI job.
- PRs that don't include the regenerated files fail the CI job.

- Op-queue & replay
- Client persists ops to IndexedDB and can replay to `/api/sync/ops` with idempotency keys.
- Server accepts op batches, validates with Zod, and returns a checkpoint.
- Integration test demonstrates offline queue replay success.

Risks & mitigations

- Risk: Forced upgrades will frustrate users. Mitigation: Use staged rollout, server-side adapters, and telemetry-driven cutoff.
- Risk: Yjs migration complexity. Mitigation: Use schemaVersion metadata and one-time server-side migration handlers.

Next immediate tasks (this sprint)

1. Generate endpoint inventory (owner: @backend-owner).
2. Implement `/api/metadata` scaffold and add to OpenAPI (owner: @backend-owner).
3. Add client boot-time check and non-blocking banner (owner: @frontend-owner).
4. Move `user`/`session` Zod schemas into `packages/shared` and update imports (owner: @shared-owner).

Where to record progress

- Use `docs/plans/local-first-sync-strategy.md` for strategy context.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Correct file path reference.

Line 69 also references docs/plans/local-first-sync-strategy.md, but the file is located at docs/audits/local-first-sync-strategy.md. Fix this broken cross-reference to match the location of the strategy document.

🔗 Proposed fix
- Use `docs/plans/local-first-sync-strategy.md` for strategy context.
+ Use `docs/audits/local-first-sync-strategy.md` for strategy context.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Use `docs/plans/local-first-sync-strategy.md` for strategy context.
- Use `docs/audits/local-first-sync-strategy.md` for strategy context.
🤖 Prompt for AI Agents
In @packages/docs/audits/local-first-roadmap.md at line 69, The cross-reference
in packages/docs/audits/local-first-roadmap.md incorrectly points to
docs/plans/local-first-sync-strategy.md; update that link/text to reference the
actual file path docs/audits/local-first-sync-strategy.md so the roadmap points
to the correct strategy document.

- Use this roadmap file for milestone status and linking PRs.
- Update the repo TODO (project board) with tickets referenced by milestone.

Questions before we start

- Do you want CI to automatically commit generated clients (and require a PR for that change), or should CI only fail and expect authors to commit regeneration output?
- Who should I list as `@backend-owner`, `@frontend-owner`, and `@sync-owner` (I can default to existing repo leads if you prefer).

---

I can now: (A) generate the endpoint inventory, (B) scaffold `/api/metadata` and add OpenAPI entry, or (C) start moving Zod schemas into `packages/shared`. Which should I do next?
118 changes: 118 additions & 0 deletions packages/docs/audits/local-first-sync-strategy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
---
title: Local-First Sync Strategy (CoRATES)
date: 2026-01-07
author: Team
---

## Summary

This document describes a pragmatic strategy to keep the `web` client and the `workers` backend in sync for a local-first application. It covers which data must stay synchronized, recommended synchronization primitives (CRDTs, event-log, snapshots), contract and versioning practices, offline/queue handling, security considerations, testing, and a small rollout roadmap.

**Goals**

- Preserve local-first UX (work offline, seamless conflict resolution for collaborative docs).
- Keep client and server types and validation in strict alignment.
- Minimize forced client upgrades while retaining a safe migration path.
- Maintain robust security (auth, SSRF, input validation) and observability for sync problems.

## Scope

- Applies to `packages/web` (client) and `packages/workers` (server) in this repo.
- Emphasizes document-level collaboration, operation queues (mutations), auth/session state, derived indexes and query caches, and attachments.

## What must stay in sync (priority order)

- Live collaborative documents and checklist state (CRDT — Yjs)
- Operation queues / mutation logs (client-driven commands)
- Auth/session & user profile state (authoritative on server)
- Query cache / persisted IndexedDB snapshots (client caches)
- Derived server-side indexes, summaries, and search
- Attachments and signed URL availability

## Recommended primitives and when to use them

- CRDTs (Yjs): document bodies, checklist items, real-time multi-user edits.
- Event-log / append-only ops: user actions that are not easy CRDTs (e.g., domain-specific commands, exports, server-only transforms).
- Snapshot + deltas: large state (search indexes, generated summaries) where streaming every op is expensive.
- REST endpoints: commands, file upload, server reconciliation, and metadata.
- Optional GraphQL: read-heavy aggregations if the UI requires highly flexible client-shaped queries. Not recommended for primary CRDT sync.

## Contracts and validation

- Keep canonical DTOs and Zod schemas in `packages/shared` and import them from both `web` and `workers`.
- Publish and maintain `openapi.json` from the `workers` package; generate TypeScript clients and runtime validators for the web build in CI.
- Server must validate every incoming payload with Zod and fail fast with clear error codes (use the existing error helpers).

## Versioning, compatibility and forcing updates

- Use semantic API versioning for breaking changes and keep changes additive-first.
- Add a small metadata endpoint `/api/metadata` or `/api/compat` that returns:
- `apiVersion` (server)
- `minSupportedClient` (semver)
- `features`/capabilities
- Client reports its `clientVersion` on requests (header) and performs a boot-time check. If the server responds that the client is too old, the client shows a blocking upgrade UI.
- Service worker & BroadcastChannel based approach: server changes `minSupportedClient` — SW detects it and broadcasts a force-reload message to all tabs. This is less disruptive than immediate hard-blocks.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hyphenate compound modifier.

Change "BroadcastChannel based approach" to "BroadcastChannel-based approach" for correct grammar.

📝 Proposed fix
- Service worker & BroadcastChannel based approach: server changes `minSupportedClient` — SW detects it and broadcasts a force-reload message to all tabs. This is less disruptive than immediate hard-blocks.
+ Service worker & BroadcastChannel-based approach: server changes `minSupportedClient` — SW detects it and broadcasts a force-reload message to all tabs. This is less disruptive than immediate hard-blocks.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Service worker & BroadcastChannel based approach: server changes `minSupportedClient` — SW detects it and broadcasts a force-reload message to all tabs. This is less disruptive than immediate hard-blocks.
- Service worker & BroadcastChannel-based approach: server changes `minSupportedClient` — SW detects it and broadcasts a force-reload message to all tabs. This is less disruptive than immediate hard-blocks.
🧰 Tools
🪛 LanguageTool

[grammar] ~54-~54: Use a hyphen to join words.
Context: ... UI. - Service worker & BroadcastChannel based approach: server changes `minSuppo...

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents
In @packages/docs/audits/local-first-sync-strategy.md at line 54, Update the
phrase "BroadcastChannel based approach" to "BroadcastChannel-based approach" in
the sentence that describes the service worker and BroadcastChannel approach so
the compound modifier is hyphenated correctly; locate the sentence containing
"Service worker & BroadcastChannel based approach" and replace that fragment
with "Service worker & BroadcastChannel-based approach".

- Server-side adapters: provide temporary acceptance of legacy payloads for a migration window, translating to the canonical model.

## Offline-first & operation queue

- Persist a local operation queue in IndexedDB (attach idempotency keys and causal metadata).
- On reconnect, replay queued operations in order to a reconciliation endpoint (server validates and returns canonical state/checkpoint).
- Provide per-op status (pending, syncing, applied, failed) and UI feedback for conflicts or retries.

## Query cache hygiene

- On auth or session changes, clear in-memory query cache (`queryClient.clear()`), clear persisted caches (`clearPersistedQueryCache()`), and trigger `session().refetch()` (already implemented in `better-auth-store`).
- Expose a `cacheVersion` server-side value in `/api/metadata`; clients can force invalidate persisted caches if `cacheVersion` changes.

## Security

- Server-side must re-validate everything (Zod), never trust client
- Keep SSRF protection for proxy endpoints (already implemented in `packages/workers/src/lib/ssrf-protection.js`).
- Require auth for mutation endpoints, and rate-limit or challenge suspicious sync clients.

## CI / Developer workflows

- Generate and commit client types from `openapi.json` during CI; fail PRs if generated types diverge.
- Add unit tests that exercise generated client against a local worker (or mocked OpenAPI responses).
- Linting and schema checks: enforce Zod schemas for incoming request validation in `workers`.

## Testing and monitoring

- Integration tests for:
- Yjs document sync via Durable Objects or WebSocket relay
- Offline queue replay and idempotency checks
- Version negotiation and forced upgrade flows
- Add telemetry for: client-version distribution, number of queued ops per client, sync failure rates, backlog size, and time-to-replay.

## Migration & rollout strategy

1. Add `minSupportedClient` to `/api/metadata` and ship server-side capability flags.
2. Add client-side boot check + non-blocking UI banner for deprecation.
3. For breaking changes: provide server-side adapter accepting old payloads for a 2–8 week window while migrating clients.
4. After the window, bump `minSupportedClient` and use SW/BroadcastChannel to force updates.

## Roadmap (concrete next steps)

1. Inventory repo: list endpoints, identify which domains use Yjs vs REST (I can generate this list).
2. Move canonical Zod DTOs to `packages/shared` and import them in server & client.
3. Add `/api/metadata` and expose `minSupportedClient` and `cacheVersion`.
4. Add CI job that regenerates client types from `openapi.json` and fails if uncommitted.
5. Implement an IndexedDB-backed op queue library and a server reconciliation endpoint.
6. Add monitoring for client-version and sync metrics.

## Risks & tradeoffs

- CRDTs (Yjs) add complexity but are the best UX for real-time document collaboration.
- GraphQL can simplify client query shapes but complicates offline persistence and cache invalidation for local-first apps.
- Forcing upgrades is disruptive; prefer staged rollout with server adapters and telemetry-driven cutoff.

## Appendix: recommended endpoints

- `GET /api/metadata` → { apiVersion, minSupportedClient, cacheVersion, features }
- `POST /api/sync/ops` → accept op batches and return { checkpoint }
- `POST /api/reconcile/doc/:id` → server reconciliation for doc snapshots

---

If you want, I can: (A) generate the endpoint inventory now, (B) move selected Zod schemas to `packages/shared`, or (C) scaffold `/api/metadata` and CI typegen. Which should I start?
Comment on lines +117 to +118
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Adopt a more formal tone in the closing section.

The opening phrase "If you want, I can:" is too informal for documentation. Use more formal language to align with the professional tone of the rest of the document.

📝 Proposed fix
- If you want, I can: (A) generate the endpoint inventory now, (B) move selected Zod schemas to `packages/shared`, or (C) scaffold `/api/metadata` and CI typegen. Which should I start?
+ Next steps: (A) generate the endpoint inventory, (B) move selected Zod schemas to `packages/shared`, or (C) scaffold `/api/metadata` and CI typegen.
🧰 Tools
🪛 LanguageTool

[style] ~117-~117: This phrasing can be overused. Try elevating your writing with a more formal alternative.
Context: ... reconciliation for doc snapshots --- If you want, I can: (A) generate the endpoint inven...

(IF_YOU_WANT)

🤖 Prompt for AI Agents
In @packages/docs/audits/local-first-sync-strategy.md around lines 117 - 118,
Replace the informal closing "If you want, I can:" with a more formal
alternative that matches the document tone; update the options to begin with a
complete sentence such as "Please indicate whether you would like me to: (A)
generate the endpoint inventory now, (B) move selected Zod schemas to
`packages/shared`, or (C) scaffold `/api/metadata` and CI typegen." Ensure
punctuation and capitalization are consistent with surrounding documentation.

20 changes: 15 additions & 5 deletions packages/web/src/components/checklist/ChecklistYjsWrapper.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createSignal, createEffect, createMemo, Show } from 'solid-js';
import { createSignal, createEffect, createMemo, Show, onCleanup } from 'solid-js';
import { useParams, useNavigate, useLocation } from '@solidjs/router';
import ChecklistWithPdf from '@/components/checklist/ChecklistWithPdf.jsx';
import useProject from '@/primitives/useProject/index.js';
Expand Down Expand Up @@ -32,6 +32,13 @@ export default function ChecklistYjsWrapper() {
const [pdfLoading, setPdfLoading] = createSignal(false);
const [selectedPdfId, setSelectedPdfId] = createSignal(null);

// Cleanup PDF data on unmount to release memory
onCleanup(() => {
setPdfData(null);
setPdfFileName(null);
setSelectedPdfId(null);
});

// Use full hook for write operations
const {
connect,
Expand Down Expand Up @@ -134,6 +141,9 @@ export default function ChecklistYjsWrapper() {
setAttemptedPdfFile(fileName);
setPdfLoading(true);

// Clear previous PDF data to release memory before loading new one
setPdfData(null);

// Try cache first, then fall back to cloud
getCachedPdf(params.projectId, params.studyId, fileName)
.then(cachedData => {
Expand Down Expand Up @@ -298,9 +308,9 @@ export default function ChecklistYjsWrapper() {

// Show confirmation dialog before marking complete
const confirmed = await confirmDialog.open({
title: 'Mark Checklist as Complete?',
title: 'Mark Appraisal as Complete?',
description:
'Once marked complete, this checklist will be locked and cannot be edited. Are you sure you want to proceed?',
'Once marked complete, this appraisal will be locked and cannot be edited. Are you sure you want to proceed?',
confirmText: 'Mark Complete',
cancelText: 'Cancel',
variant: 'warning',
Expand All @@ -315,8 +325,8 @@ export default function ChecklistYjsWrapper() {
const statusLabel =
nextStatus === CHECKLIST_STATUS.FINALIZED ? 'completed' : 'awaiting reconciliation';
showToast.success(
'Checklist Completed',
`This checklist has been marked as ${statusLabel} and is now locked.`,
'Appraisal Completed',
`This appraisal has been marked as ${statusLabel} and is now locked.`,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export function ScoringSummary(props) {
*/
function ResourcesDialog(props) {
return (
<Dialog open={props.open} onOpenChange={open => !open && props.onClose()}>
<Dialog.Root open={props.open} onOpenChange={open => !open && props.onClose()}>
<Dialog.Backdrop class='fixed inset-0 z-40 bg-black/50' />
<Dialog.Positioner class='fixed inset-0 z-50 flex items-center justify-center p-4'>
<Dialog.Content class='max-h-[85vh] w-full max-w-md overflow-auto rounded-lg bg-white shadow-xl'>
Expand Down Expand Up @@ -239,7 +239,7 @@ function ResourcesDialog(props) {
</div>
</Dialog.Content>
</Dialog.Positioner>
</Dialog>
</Dialog.Root>
);
}

Expand Down
29 changes: 25 additions & 4 deletions packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { DocumentPasswordPrompt } from './components/document-password-prompt';
import { SearchSidebar } from './components/search-sidebar';
import { ThumbnailsSidebar } from './components/thumbnails-sidebar';
import { PageControls } from './components/page-controls';
import { ConsoleLogger } from '@embedpdf/models';
// import { ConsoleLogger } from '@embedpdf/models';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check ConsoleLogger usage across entire codebase
rg "ConsoleLogger" --type=ts --type=tsx

Repository: InfinityBowman/corates

Length of output: 93


🏁 Script executed:

#!/bin/bash
# Also check if this is a common pattern with commented debug code
rg "// import.*from.*embedpdf" --type=ts --type=tsx

Repository: InfinityBowman/corates

Length of output: 93


🏁 Script executed:

#!/bin/bash
# Check the actual file to see the context of the commented lines
if [ -f "packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx" ]; then
  echo "=== Lines 40-60 ==="
  sed -n '40,60p' packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx
  echo -e "\n=== Lines 80-90 ==="
  sed -n '80,90p' packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx
  echo -e "\n=== Lines 270-280 ==="
  sed -n '270,280p' packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx
else
  echo "File not found"
fi

Repository: InfinityBowman/corates

Length of output: 1808


🏁 Script executed:

#!/bin/bash
# Search for any active (non-commented) usage of ConsoleLogger
grep -r "ConsoleLogger" packages/web/src --include="*.tsx" --include="*.ts" | grep -v "^[^:]*://.*//.*ConsoleLogger"

Repository: InfinityBowman/corates

Length of output: 283


🏁 Script executed:

#!/bin/bash
# Check if ConsoleLogger is imported anywhere without comment
find packages/web/src -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "ConsoleLogger" {} \;

Repository: InfinityBowman/corates

Length of output: 128


Delete commented ConsoleLogger lines per coding guidelines.

The ConsoleLogger import and all references (lines 44, 58, 85, 274) are completely unused and commented out. Remove these lines entirely rather than leaving obsolete code paths in comments, per the guideline that states: "Do NOT reference removed or obsolete code paths in comments."

🤖 Prompt for AI Agents
In @packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx at line 44,
Remove the commented-out ConsoleLogger import and all commented references to
ConsoleLogger in this file (delete the "// import { ConsoleLogger } from
'@embedpdf/models';" and any other commented ConsoleLogger usages), leaving no
obsolete commented code paths referencing ConsoleLogger; ensure no other code
depends on it and run a quick build to confirm no unused symbol warnings remain.

import { AnnotationSelectionMenu } from './components/annotation-selection-menu';
import { SelectionSelectionMenu } from './components/selection-selection-menu';
import { EmptyState } from './components/empty-state';
Expand All @@ -55,7 +55,7 @@ import {
paramResolvers,
} from './config';

const logger = new ConsoleLogger();
// const logger = new ConsoleLogger();

// Type for tracking sidebar state per document
type SidebarState = {
Expand All @@ -82,7 +82,7 @@ export function ViewerPage({
}: ViewerPageProps = {}) {
const containerRef = useRef<HTMLDivElement>(null);
const { engine, isLoading, error } = usePdfiumEngine({
logger,
// logger,
});

// Track sidebar state per document
Expand Down Expand Up @@ -233,6 +233,27 @@ export function ViewerPage({
loadDocument();
}, [pdfData, selectedPdfId, pdfFileName, pdfs]);

// Cleanup: close the active document and release resources on unmount
useEffect(() => {
return () => {
const closeActiveDocument = async () => {
if (docManagerRef.current && activeDocumentIdRef.current) {
try {
await docManagerRef.current.closeDocument(activeDocumentIdRef.current);
} catch (err) {
console.warn('Error closing document on unmount:', err);
}
}
// Clear refs to release memory
docManagerRef.current = null;
activeDocumentIdRef.current = null;
previousSelectedPdfIdRef.current = undefined;
isLoadingRef.current = false;
};
closeActiveDocument();
};
}, []);
Comment on lines +236 to +255
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix async cleanup function in useEffect.

The cleanup function returned by useEffect must be synchronous, but closeActiveDocument is async and called without await. This means the document close operation won't complete before unmount, potentially causing memory leaks or orphaned resources in the PDF engine.

🐛 Proposed fix for synchronous cleanup

The cleanup should fire-and-forget the async operation, acknowledging it may not complete:

-  // Cleanup: close the active document and release resources on unmount
   useEffect(() => {
     return () => {
-      const closeActiveDocument = async () => {
-        if (docManagerRef.current && activeDocumentIdRef.current) {
-          try {
-            await docManagerRef.current.closeDocument(activeDocumentIdRef.current);
-          } catch (err) {
-            console.warn('Error closing document on unmount:', err);
-          }
-        }
-        // Clear refs to release memory
-        docManagerRef.current = null;
-        activeDocumentIdRef.current = null;
-        previousSelectedPdfIdRef.current = undefined;
-        isLoadingRef.current = false;
-      };
-      closeActiveDocument();
+      // Fire async close without awaiting (may not complete before unmount)
+      if (docManagerRef.current && activeDocumentIdRef.current) {
+        docManagerRef.current.closeDocument(activeDocumentIdRef.current)
+          .catch(err => console.warn('Error closing document on unmount:', err));
+      }
+      // Clear refs immediately
+      docManagerRef.current = null;
+      activeDocumentIdRef.current = null;
+      previousSelectedPdfIdRef.current = undefined;
+      isLoadingRef.current = false;
     };
   }, []);
🤖 Prompt for AI Agents
In @packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx around lines
236 - 255, The cleanup returned from useEffect is currently calling an async
closeActiveDocument without awaiting, which makes the cleanup itself async; make
the cleanup synchronous by defining the async function (closeActiveDocument)
inside the effect but invoking it in a fire-and-forget manner (for example: void
closeActiveDocument(); or closeActiveDocument().catch(err =>
console.warn(...))). Ensure you still call
docManagerRef.current.closeDocument(activeDocumentIdRef.current) inside that
async function and preserve the ref-clearing (docManagerRef,
activeDocumentIdRef, previousSelectedPdfIdRef, isLoadingRef) and error logging,
but do not return an async function from useEffect.


if (error) {
return <div>Error: {error.message}</div>;
}
Expand All @@ -250,7 +271,7 @@ export function ViewerPage({
<div className='flex flex-1 flex-col overflow-hidden select-none'>
<EmbedPDF
engine={engine}
logger={logger}
// logger={logger}
plugins={plugins}
onInitialized={async registry => {
const docManager = registry
Expand Down
Loading