Types and animations#321
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | 799a306 | Commit Preview URL Branch Preview URL |
Jan 24 2026, 04:44 AM |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds SVG globals to ESLint, removes the Dashboard component, adds many Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/components/pdf/embedpdf/preact/src/components/thumbnails-sidebar.tsx (1)
5-10: TheonCloseprop is defined but never used.The prop is aliased to
_onClose(indicating it's unused) but there's no close button or callback invocation anywhere in the component. If this is intentional placeholder for future functionality, consider adding a TODO comment. Otherwise, remove the unused prop from the type definition to avoid confusion.Option A: Remove unused prop if not needed
type ThumbnailsSidebarProps = { documentId: string; - onClose?: () => void; }; -export function ThumbnailsSidebar({ documentId, onClose: _onClose }: ThumbnailsSidebarProps) { +export function ThumbnailsSidebar({ documentId }: ThumbnailsSidebarProps) {Option B: Add TODO if planned for future use
type ThumbnailsSidebarProps = { documentId: string; + // TODO(agent): Implement close button in sidebar header onClose?: () => void; };packages/web/src/components/pdf/embedpdf/preact/src/components/pdf-picker.tsx (1)
15-28: Hooks called after early return violates Rules of Hooks.
useMemoon line 21 is called after the early return on lines 16-18. In React/Preact, hooks must be called unconditionally at the top level of the component - the order and number of hook calls must be consistent across renders.Proposed fix: move hook before conditional return
export function PdfPicker({ pdfs, selectedPdfId, onPdfSelect }: PdfPickerProps) { const [isMenuOpen, setIsMenuOpen] = useState(false); + // Sort PDFs: primary first, then protocol, then secondary + const sortedPdfs = useMemo(() => { + if (!pdfs || pdfs.length <= 1) return []; + return [...pdfs].sort((a, b) => { + const tagOrder = { primary: 0, protocol: 1, secondary: 2 }; + const tagA = tagOrder[a.tag as keyof typeof tagOrder] ?? 2; + const tagB = tagOrder[b.tag as keyof typeof tagOrder] ?? 2; + return tagA - tagB; + }); + }, [pdfs]); + // Don't show if there's only one or no PDFs - if (!pdfs || pdfs.length <= 1) { + if (!pdfs || pdfs.length <= 1 || sortedPdfs.length === 0) { return null; } - - // Sort PDFs: primary first, then protocol, then secondary - const sortedPdfs = useMemo(() => { - return [...pdfs].sort((a, b) => { - const tagOrder = { primary: 0, protocol: 1, secondary: 2 }; - const tagA = tagOrder[a.tag as keyof typeof tagOrder] ?? 2; - const tagB = tagOrder[b.tag as keyof typeof tagOrder] ?? 2; - return tagA - tagB; - }); - }, [pdfs]);
🤖 Fix all issues with AI agents
In `@packages/web/src/components/pdf/embedpdf/preact/tsconfig.json`:
- Around line 22-28: Update tsconfig.json so baseUrl points seven levels up
instead of five: change the "baseUrl" value (currently "../../../../..") to use
seven "../" segments (i.e., "../" repeated 7) so the "paths" entries for
"react", "react-dom", and "react/jsx-runtime" resolve to the monorepo root
node_modules/preact packages.
In `@packages/web/tsconfig.json`:
- Around line 37-42: The tsconfig.json exclude list contains a non-existent path
"src/components/pdf/embedpdf/preact-2"; open the "exclude" array in the
tsconfig.json and remove the "src/components/pdf/embedpdf/preact-2" entry so
only real directories (e.g., "node_modules", "dist",
"src/components/pdf/embedpdf/preact") remain.
♻️ Duplicate comments (4)
packages/web/src/components/pdf/embedpdf/preact/src/components/redaction-selection-menu.tsx (1)
1-1: Same concern: file-wide@ts-nocheck.Please scope suppression to specific lines and document the rationale with TODO(agent) and an issue reference.
packages/web/src/components/pdf/embedpdf/preact/src/components/ui/toolbar-button.tsx (1)
1-1: Same concern: file-wide@ts-nocheck.Please scope suppression to specific lines and document the rationale with TODO(agent) and an issue reference.
packages/web/src/components/pdf/embedpdf/preact/src/components/selection-selection-menu.tsx (1)
1-1: Same concern: file-wide@ts-nocheck.Please scope suppression to specific lines and document the rationale with TODO(agent) and an issue reference.
packages/web/src/components/pdf/embedpdf/preact/src/components/search-sidebar.tsx (1)
1-1: Same concern: file-wide@ts-nocheck.Please scope suppression to specific lines and document the rationale with TODO(agent) and an issue reference.
🧹 Nitpick comments (15)
packages/web/src/components/pdf/embedpdf/preact/src/components/thumbnails-sidebar.tsx (1)
1-1:@ts-nochecksuppresses all type checking for this file.While this may be a deliberate decision for handling Preact/TypeScript compatibility across multiple files in this PR, blanket suppression can hide real type errors. Consider using targeted
@ts-expect-erroror@ts-ignorecomments on specific problematic lines instead, or addressing the underlying type issues if feasible.packages/web/src/components/pdf/embedpdf/preact/src/components/document-password-prompt.tsx (2)
1-1: Consider the trade-offs of blanket@ts-nocheck.While the AI summary indicates this is part of a broader pattern across PDF Preact components, disabling TypeScript checking entirely hides potential type errors. If this is a transitional measure for React-to-Preact compatibility, consider adding a
TODO(agent):comment explaining the intent and when proper typing should be restored.
93-93: Type cast is good practice, but redundant with@ts-nocheck.The explicit cast to
HTMLInputElementis a clean fix for accessing the input value. However, with@ts-nocheckat line 1, this cast has no effect at compile time. If you're keeping@ts-nocheck, this cast is unnecessary. If you want type safety, consider removing@ts-nocheckand properly typing the component instead.packages/web/src/components/pdf/embedpdf/preact/src/components/zoom-toolbar.tsx (1)
1-1://@ts-nocheck`` disables all type safety for this file.This directive is part of the broader PR pattern to suppress TypeScript checks across the preact PDF components. While this unblocks the build, it also masks potential type errors. Consider tracking this as technical debt to revisit once the React-to-Preact aliasing is fully stabilized, or use more targeted
//@ts-expect-error`` comments for specific problematic lines.packages/web/src/components/mocks/MockIndex.jsx (1)
116-119: Section renamed but content is empty.The heading was changed from "Other Mocks" to "Components", but the grid below it is empty. If this section is intentionally a placeholder for future content, consider adding a brief comment or removing it until content is added.
packages/web/src/components/pdf/embedpdf/preact/src/components/command-button.tsx (1)
56-81: Consider extracting the variant rendering logic.The nested ternary operators are functional but reduce readability. Extracting this to a separate function or using a switch/object lookup could improve maintainability.
Example refactor using a render function
+const renderContent = () => { + switch (variant) { + case 'text': + return <span className='text-sm'>{command.label}</span>; + case 'icon-text': + return ( + <> + {IconComponent && ( + <IconComponent + className={twMerge('mr-2 h-5 w-5', iconProps.className)} + title={command.label} + style={{ color: iconProps.primaryColor }} + /> + )} + <span>{command.label}</span> + </> + ); + case 'tab': + return <span className='px-3 py-1'>{command.label}</span>; + default: + return IconComponent ? ( + <IconComponent + className={twMerge('h-5 w-5', iconProps.className)} + title={command.label} + style={{ color: iconProps.primaryColor, fill: iconProps.secondaryColor }} + /> + ) : ( + <span>{command.label}</span> + ); + } +};packages/web/src/components/pdf/embedpdf/preact/src/components/viewer-toolbar.tsx (1)
90-99: Consider removing or documenting the commented-out redact button.The
redactmode is still defined inViewModeandRedactionToolbarrenders whenmode === 'redact', but the UI button to activate it is commented out. If this is intentionally disabled, consider adding a// TODO(agent): ...comment explaining why, or remove the dead code if the feature is fully deferred.packages/web/src/components/pdf/embedpdf/preact/src/components/page-controls.tsx (2)
1-1: Avoid file-wide TypeScript suppression.
A module-level@ts-nocheckhides all typing regressions. Prefer targeted@ts-expect-erroror fix the specific typings.
120-122: PrefercurrentTargetto avoid the cast.
Usinge.currentTargetkeeps the typing safe without assertinge.target.Proposed change
- const value = (e.target as HTMLInputElement).value.replace(/[^0-9]/g, ''); + const value = e.currentTarget.value.replace(/[^0-9]/g, '');packages/web/src/components/pdf/embedpdf/preact/src/components/ui/button.tsx (1)
1-1: Avoid file-wide TypeScript suppression.
A full@ts-nocheckdisables type safety for this shared component. Consider narrowing to specific lines instead.packages/web/src/components/pdf/embedpdf/preact/src/components/tab-context-menu.tsx (1)
1-1: Avoid file-wide TypeScript suppression.
If this is temporary, prefer a targeted suppression so other checks still run.packages/web/src/components/pdf/embedpdf/preact/src/viewer.tsx (1)
1-1: Avoid file-wide TypeScript suppression in this large module.
Given the size and complexity here, a full@ts-nocheckcan hide regressions. Prefer targeted suppressions or fix the specific type issues.packages/web/src/components/pdf/embedpdf/preact/src/components/ui/dialog-content.tsx (1)
1-1: Consider targeted suppression instead of file-wide@ts-nocheck.While I understand this is part of a broader pattern across the PDF/embedpdf Preact components,
@ts-nocheckdisables all type checking, masking potential issues and reducing type safety benefits. Consider either:
- Fixing the underlying type incompatibilities (likely React-to-Preact type mismatches)
- Using targeted
//@ts-ignore`` comments only where neededFor a simple component like this one, the types appear straightforward and may not even require suppression.
packages/web/src/components/pdf/embedpdf/preact/src/components/print-dialog.tsx (2)
1-1: File-wide@ts-nocheckmakes explicit type casts redundant.With
@ts-nocheckat line 1, the explicit(e.target as HTMLInputElement)casts on lines 90, 102, 116, 128, and 147 serve no purpose since TypeScript isn't checking this file anyway.Consider either:
- Removing
@ts-nocheckand keeping the explicit casts (preferred - maintains type safety)- Keeping
@ts-nocheckand removing the now-redundant casts (less ideal but consistent)The current state has the maintenance burden of both approaches with the benefits of neither.
57-74: Print failure provides no user feedback.When printing fails, the error is only logged to console. Users won't know why the print dialog remains open or appears stuck.
Consider adding user-visible error feedback:
Suggested improvement
+ const [error, setError] = useState<string | null>(null); // Reset form when dialog opens/closes useEffect(() => { if (!isOpen) { setSelection('all'); setCustomPages(''); setIncludeAnnotations(true); setIsLoading(false); + setError(null); } }, [isOpen]); // In handlePrint: try { const task = print.print(options); if (task) { task.wait( () => { onClose(); }, error => { console.error('Print failed:', error); setIsLoading(false); + setError('Print failed. Please try again.'); }, ); } } catch (err) { console.error('Print failed:', err); setIsLoading(false); + setError('Print failed. Please try again.'); }Then render the error in the dialog content.
| "allowSyntheticDefaultImports": true, | ||
| "baseUrl": "../../../../..", | ||
| "paths": { | ||
| "react": ["node_modules/preact/compat"], | ||
| "react-dom": ["node_modules/preact/compat"], | ||
| "react/jsx-runtime": ["node_modules/preact/jsx-runtime"] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check repository structure and locate the tsconfig file
git ls-files | grep -E "(packages/web/src/components/pdf|tsconfig)" | head -20Repository: InfinityBowman/corates
Length of output: 1472
🏁 Script executed:
# Verify the actual tsconfig.json file exists and read its full content
if [ -f "packages/web/src/components/pdf/embedpdf/preact/tsconfig.json" ]; then
echo "=== File exists, reading content ==="
cat -n "packages/web/src/components/pdf/embedpdf/preact/tsconfig.json"
else
echo "File not found at expected location"
# Try to find it
find . -name "tsconfig.json" -path "*embedpdf*preact*" 2>/dev/null
fiRepository: InfinityBowman/corates
Length of output: 1217
🏁 Script executed:
# Check directory structure to verify baseUrl path calculation
# From packages/web/src/components/pdf/embedpdf/preact/, ../../../../.. should resolve to:
python3 << 'EOF'
from pathlib import Path
import os
# Starting point
tsconfig_dir = Path("packages/web/src/components/pdf/embedpdf/preact")
print(f"tsconfig location: {tsconfig_dir}")
print(f"Depth from root: {len(tsconfig_dir.parts)}")
# Apply baseUrl ../../../../..
resolved = tsconfig_dir
for _ in range(5):
resolved = resolved.parent
print(f"baseUrl '../../../../..' resolves to: {resolved}")
print(f"Path relative to repo root: {resolved.relative_to('.')}")
# Check where node_modules typically are
print(f"\nCommon node_modules locations:")
for potential_nm in ["node_modules", "packages/web/node_modules", "packages/node_modules"]:
if Path(potential_nm).exists():
print(f" ✓ {potential_nm} exists")
else:
print(f" ✗ {potential_nm} does not exist")
EOFRepository: InfinityBowman/corates
Length of output: 152
🏁 Script executed:
# Check if the referenced preact packages exist in node_modules
echo "=== Checking preact references ==="
if [ -d "node_modules" ]; then
echo "node_modules at repo root:"
ls -la node_modules | grep preact
elif [ -d "packages/web/node_modules" ]; then
echo "node_modules at packages/web:"
ls -la packages/web/node_modules | grep preact
else
echo "Checking for preact in package.json files:"
rg "preact" packages/web/package.json --context 2
fiRepository: InfinityBowman/corates
Length of output: 127
🏁 Script executed:
# Check the actual workspace structure and whether path mapping would work
echo "=== Package structure ==="
ls -la packages/web/src/components/pdf/embedpdf/ 2>/dev/null || echo "Directory structure not found"
echo ""
echo "=== Parent tsconfig.json files if any ==="
find packages/web/src/components/pdf -name "tsconfig.json" 2>/dev/nullRepository: InfinityBowman/corates
Length of output: 431
🏁 Script executed:
# Check where preact is actually installed in the monorepo
echo "=== Checking package.json for preact dependencies ==="
rg "preact" packages/web/package.json -A 2 -B 2
echo -e "\n=== Checking root package.json ==="
if [ -f "package.json" ]; then
rg "preact" package.json -A 2 -B 2
fi
echo -e "\n=== Verify correct baseUrl depth from tsconfig location ==="
python3 << 'EOF'
from pathlib import Path
tsconfig_path = Path("packages/web/src/components/pdf/embedpdf/preact")
print(f"tsconfig directory: {tsconfig_path}")
print(f"Parts: {list(tsconfig_path.parts)}")
print(f"Depth: {len(tsconfig_path.parts)} levels from root")
# To reach repo root
levels_to_root = len(tsconfig_path.parts)
print(f"\nTo reach repo root from tsconfig location:")
print(f" Need to go up {levels_to_root} levels")
print(f" baseUrl should be: {''.join(['../' for _ in range(levels_to_root)])}")
# Current baseUrl analysis
current_baseurl = "../../../../.."
levels_up = current_baseurl.count("..")
resolved_from = Path("packages/web/src/components/pdf/embedpdf/preact")
for _ in range(levels_up):
resolved_from = resolved_from.parent
print(f"\nCurrent baseUrl '{current_baseurl}':")
print(f" Goes up {levels_up} levels")
print(f" Resolves to: {resolved_from}")
EOFRepository: InfinityBowman/corates
Length of output: 152
🏁 Script executed:
# Verify node_modules structure
echo "=== Monorepo node_modules structure ==="
if [ -d "node_modules/preact" ]; then
echo "✓ Found preact at repo root node_modules"
ls -la node_modules/preact | head -10
elif [ -d "packages/web/node_modules/preact" ]; then
echo "✓ Found preact at packages/web/node_modules"
else
echo "Checking installed packages:"
find . -type d -name "preact" -not -path "*/src/*" 2>/dev/null | head -5
fiRepository: InfinityBowman/corates
Length of output: 134
🏁 Script executed:
# Try simple cat to read package.json instead of rg (which timed out)
echo "=== packages/web/package.json dependencies ==="
if [ -f "packages/web/package.json" ]; then
cat packages/web/package.json | grep -A 5 -B 5 "preact" || echo "No preact found in packages/web/package.json"
fi
echo -e "\n=== root package.json dependencies ==="
if [ -f "package.json" ]; then
cat package.json | grep -A 5 -B 5 "preact" || echo "No preact found in root package.json"
fiRepository: InfinityBowman/corates
Length of output: 812
🏁 Script executed:
# Check the actual depth calculation and verify if node_modules is accessible
python3 << 'EOF'
from pathlib import Path
import os
print("=== Path Resolution Analysis ===\n")
# Calculate where baseUrl points
tsconfig_dir = Path("packages/web/src/components/pdf/embedpdf/preact")
print(f"tsconfig.json is at: {tsconfig_dir}")
print(f"Relative path parts from root: {len(tsconfig_dir.parts)}")
# Current baseUrl
current_base = "../../../../.."
levels = current_base.count("..")
resolved = tsconfig_dir
for i in range(levels):
resolved = resolved.parent
print(f"\nbaseUrl '{current_base}' (5 levels up) resolves to:")
print(f" {resolved} (relative path: {resolved.relative_to('.') if resolved.relative_to('.') != Path('.') else 'repo root'})")
# Paths mapping
print(f"\nPath mappings would resolve to:")
print(f" 'react' → {resolved}/node_modules/preact/compat")
print(f" 'react-dom' → {resolved}/node_modules/preact/compat")
print(f" 'react/jsx-runtime' → {resolved}/node_modules/preact/jsx-runtime")
# Check if these paths exist
print(f"\nVerifying if resolved paths exist:")
for subpath in ["node_modules/preact/compat", "node_modules/preact/jsx-runtime"]:
full_path = resolved / subpath
exists = "✓" if full_path.exists() else "✗"
print(f" {exists} {full_path}")
EOFRepository: InfinityBowman/corates
Length of output: 638
🏁 Script executed:
# Check if this is a monorepo with workspaces
echo "=== Checking monorepo configuration ==="
if [ -f "pnpm-workspace.yaml" ]; then
echo "pnpm workspaces:"
cat pnpm-workspace.yaml
elif [ -f "turbo.json" ]; then
echo "Turbo config:"
head -20 turbo.json
else
echo "Checking package.json for workspaces:"
cat package.json | grep -A 10 '"workspaces"' || echo "No workspace config found in root package.json"
fiRepository: InfinityBowman/corates
Length of output: 293
🏁 Script executed:
# Direct check: does preact exist in packages/web/node_modules?
ls -la packages/web/node_modules 2>/dev/null | grep -i preact || echo "No preact in packages/web/node_modules"Repository: InfinityBowman/corates
Length of output: 104
🏁 Script executed:
# Check if preact is at the repo root node_modules (as expected in pnpm monorepo)
echo "=== Checking root node_modules for preact ==="
if [ -d "node_modules/preact" ]; then
echo "✓ Found preact at repo root node_modules"
echo ""
echo "Package structure:"
ls -la node_modules/preact/compat 2>/dev/null || echo " preact/compat directory exists: $([ -d 'node_modules/preact/compat' ] && echo 'YES' || echo 'NO')"
ls -la node_modules/preact/jsx-runtime 2>/dev/null || echo " preact/jsx-runtime directory exists: $([ -d 'node_modules/preact/jsx-runtime' ] && echo 'YES' || echo 'NO')"
else
echo "✗ preact NOT found at repo root node_modules"
echo ""
echo "Checking node_modules directory:"
ls -d node_modules 2>/dev/null && echo "node_modules exists at root" || echo "node_modules does not exist at root"
fiRepository: InfinityBowman/corates
Length of output: 231
🏁 Script executed:
# Verify the correct baseUrl depth needed for pnpm root node_modules
python3 << 'EOF'
from pathlib import Path
tsconfig_dir = Path("packages/web/src/components/pdf/embedpdf/preact")
repo_root = Path(".")
# Calculate correct levels to reach repo root
current = tsconfig_dir
levels_needed = 0
while current != repo_root:
current = current.parent
levels_needed += 1
print(f"Correct baseUrl depth to reach repo root:")
print(f" Need to go up {levels_needed} levels")
print(f" Correct baseUrl should be: {''.join(['../' for _ in range(levels_needed)])}")
print(f"\nCurrent baseUrl analysis:")
current_base = "../../../../.."
current_levels = current_base.count("..")
print(f" Current: '{current_base}' ({current_levels} levels)")
print(f" ERROR: Currently goes up {current_levels} levels, but should go up {levels_needed}")
print(f" Missing {levels_needed - current_levels} level(s)")
print(f"\nWith correct baseUrl, paths would resolve to:")
print(f" 'react' → node_modules/preact/compat")
print(f" 'react-dom' → node_modules/preact/compat")
print(f" 'react/jsx-runtime' → node_modules/preact/jsx-runtime")
EOFRepository: InfinityBowman/corates
Length of output: 511
Fix baseUrl depth: should be 7 levels, not 5, to reach repo root node_modules.
The baseUrl: "../../../../.." (5 levels) resolves to packages/web/, but preact packages are hoisted at the repo root node_modules in this pnpm monorepo. The paths then incorrectly point to nonexistent packages/web/node_modules/preact/compat and packages/web/node_modules/preact/jsx-runtime.
Change baseUrl to "..../../../../../../" (7 levels) so paths resolve to the correct node_modules/preact/compat and node_modules/preact/jsx-runtime at repo root.
🤖 Prompt for AI Agents
In `@packages/web/src/components/pdf/embedpdf/preact/tsconfig.json` around lines
22 - 28, Update tsconfig.json so baseUrl points seven levels up instead of five:
change the "baseUrl" value (currently "../../../../..") to use seven "../"
segments (i.e., "../" repeated 7) so the "paths" entries for "react",
"react-dom", and "react/jsx-runtime" resolve to the monorepo root
node_modules/preact packages.
1 participant
Summary by CodeRabbit
Refactor
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.