FE-594: Fix PDFDocumentProxy type mismatch in pdf-preview#8636
FE-594: Fix PDFDocumentProxy type mismatch in pdf-preview#8636TimDiekmann merged 1 commit intomainfrom
Conversation
`react-pdf` 9.2.1 bundles its own `pdfjs-dist` 4.8.69 whose
`PDFDocumentProxy` type has diverged from the hoisted copy that the
direct `import type { PDFDocumentProxy } from "pdfjs-dist"` resolves
to. Replace with react-pdf's `DocumentCallback` alias which points at
its own bundled version.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit 83d9f4b. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: Resolves a TypeScript type mismatch in the PDF preview/search components caused by divergent 🤖 Was this summary useful? React with 👍 or 👎 |
alex-e-leon
left a comment
There was a problem hiding this comment.
LGTM. Next steps sound great too.
🌟 What is the purpose of this PR?
Fix a
tscerror inpdf-preview.tsx/pdf-search.tsxcaused byPDFDocumentProxytype divergence between the hoistedpdfjs-distand the version bundled insidereact-pdf.🔗 Related links
🚫 Blocked by
Nothing.
🔍 What does this change?
import type { PDFDocumentProxy } from "pdfjs-dist"withDocumentCallbackfromreact-pdf/dist/cjs/shared/typesin bothpdf-preview.tsxandpdf-search.tsx.DocumentCallbackis react-pdf's own alias forPDFDocumentProxy, guaranteed to match its bundled pdfjs-dist version.pdfjs-distwas never a declared dependency of@apps/hash-frontend— the import only resolved via yarn hoisting.import/no-extraneous-dependenciesdidn't catch it because the rule is only scoped to test/storybook/config files in the base eslint config.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
pdfjs-distis still not a declared dependency —TextIteminpdf-search.tsxstill imports frompdfjs-dist/types/src/display/api(react-pdf v9 doesn't re-export it). Adding it explicitly or bumping to react-pdf v10 (which does re-exportTextItem) are follow-up options.import/no-extraneous-dependenciesis not active for regular source files in the base eslint config — only for tests, storybooks, and config files. This allowed the undeclared import to go unnoticed.🐾 Next steps
pdfjs-distas an explicit dependency pinned to react-pdf's version.import/no-extraneous-dependenciesglobally in the eslint base config.🛡 What tests cover this?
tsc --noEmit(the build would have failed without this fix)❓ How to test this?
cd apps/hash-frontend && yarn lint:tscpdf-preview.tsxorpdf-search.tsx.