Skip to content

Commit

Permalink
Fix Outline, Page and Thumbnail components crashing when placed outsi…
Browse files Browse the repository at this point in the history
…de Document

Closes #1709
  • Loading branch information
wojtekmaj committed Feb 1, 2024
1 parent cf5327b commit e339525
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 40 deletions.
2 changes: 1 addition & 1 deletion packages/react-pdf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ Loads a document passed using `file` prop.

### Page

Displays a page. Should be placed inside `<Document />`. Alternatively, it can have `pdf` prop passed, which can be obtained from `<Document />`'s `onLoadSuccess` callback function, however some advanced functions like linking between pages inside a document may not be working correctly.
Displays a page. Should be placed inside `<Document />`. Alternatively, it can have `pdf` prop passed, which can be obtained from `<Document />`'s `onLoadSuccess` callback function, however some advanced functions like rendering annotations and linking between pages inside a document may not be working correctly.

#### Props

Expand Down
14 changes: 12 additions & 2 deletions packages/react-pdf/src/Outline.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('Outline', () => {
});

describe('loading', () => {
it('loads an outline and calls onLoadSuccess callback properly', async () => {
it('loads an outline and calls onLoadSuccess callback properly when placed inside Document', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } = makeAsyncCallback();

renderWithContext(<Outline onLoadSuccess={onLoadSuccess} />, { pdf });
Expand All @@ -68,6 +68,16 @@ describe('Outline', () => {
await expect(onLoadSuccessPromise).resolves.toMatchObject([desiredLoadedOutline]);
});

it('loads an outline and calls onLoadSuccess callback properly when pdf prop is passed', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } = makeAsyncCallback();

render(<Outline onLoadSuccess={onLoadSuccess} pdf={pdf} />);

expect.assertions(1);

await expect(onLoadSuccessPromise).resolves.toMatchObject([desiredLoadedOutline]);
});

it('calls onLoadError when failed to load an outline', async () => {
const { func: onLoadError, promise: onLoadErrorPromise } = makeAsyncCallback();

Expand Down Expand Up @@ -99,7 +109,7 @@ describe('Outline', () => {
await expect(onLoadSuccessPromise2).resolves.toMatchObject([desiredLoadedOutline2]);
});

it('throws an error when placed outside Document', () => {
it('throws an error when placed outside Document without pdf prop passed', () => {
muteConsole();

expect(() => render(<Outline />)).toThrow();
Expand Down
16 changes: 9 additions & 7 deletions packages/react-pdf/src/Outline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ export type OutlineProps = {
const Outline: React.FC<OutlineProps> = function Outline(props) {
const documentContext = useDocumentContext();

invariant(
documentContext,
'Unable to find Document context. Did you wrap <Outline /> in <Document />?',
);

const mergedProps = { ...documentContext, ...props };
const {
className,
Expand All @@ -85,7 +80,10 @@ const Outline: React.FC<OutlineProps> = function Outline(props) {
...otherProps
} = mergedProps;

invariant(pdf, 'Attempted to load an outline, but no document was specified.');
invariant(
pdf,
'Attempted to load an outline, but no document was specified. Wrap <Outline /> in a <Document /> or pass explicit `pdf` prop.',
);

const [outlineState, outlineDispatch] = useResolver<PDFOutline | null>();
const { value: outline, error: outlineError } = outlineState;
Expand Down Expand Up @@ -189,7 +187,11 @@ const Outline: React.FC<OutlineProps> = function Outline(props) {
return (
<ul>
{outline.map((item, itemIndex) => (
<OutlineItem key={typeof item.dest === 'string' ? item.dest : itemIndex} item={item} />
<OutlineItem
key={typeof item.dest === 'string' ? item.dest : itemIndex}
item={item}
pdf={pdf}
/>
))}
</ul>
);
Expand Down
19 changes: 12 additions & 7 deletions packages/react-pdf/src/OutlineItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,23 @@ type PDFOutlineItem = PDFOutline[number];

type OutlineItemProps = {
item: PDFOutlineItem;
pdf?: PDFDocumentProxy | false;
};

export default function OutlineItem(props: OutlineItemProps) {
const documentContext = useDocumentContext();

invariant(
documentContext,
'Unable to find Document context. Did you wrap <Outline /> in <Document />?',
);

const outlineContext = useOutlineContext();

invariant(outlineContext, 'Unable to find Outline context.');

const mergedProps = { ...documentContext, ...outlineContext, ...props };
const { item, linkService, onItemClick, pdf, ...otherProps } = mergedProps;

invariant(pdf, 'Attempted to load an outline, but no document was specified.');
invariant(
pdf,
'Attempted to load an outline, but no document was specified. Wrap <Outline /> in a <Document /> or pass explicit `pdf` prop.',
);

const getDestination = useCachedValue(() => {
if (typeof item.dest === 'string') {
Expand Down Expand Up @@ -64,6 +63,11 @@ export default function OutlineItem(props: OutlineItemProps) {
function onClick(event: React.MouseEvent<HTMLAnchorElement>) {
event.preventDefault();

invariant(
onItemClick || linkService,
'Either onItemClick callback or linkService must be defined in order to navigate to an outline item.',
);

if (onItemClick) {
Promise.all([getDestination(), getPageIndex(), getPageNumber()]).then(
([dest, pageIndex, pageNumber]) => {
Expand All @@ -74,7 +78,7 @@ export default function OutlineItem(props: OutlineItemProps) {
});
},
);
} else {
} else if (linkService) {
linkService.goToDestination(item.dest);
}
}
Expand All @@ -92,6 +96,7 @@ export default function OutlineItem(props: OutlineItemProps) {
<OutlineItem
key={typeof subitem.dest === 'string' ? subitem.dest : subitemIndex}
item={subitem}
pdf={pdf}
{...otherProps}
/>
))}
Expand Down
21 changes: 19 additions & 2 deletions packages/react-pdf/src/Page.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('Page', () => {
});

describe('loading', () => {
it('loads a page and calls onLoadSuccess callback properly', async () => {
it('loads a page and calls onLoadSuccess callback properly when placed inside Document', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } = makeAsyncCallback();

renderWithContext(<Page onLoadSuccess={onLoadSuccess} pageIndex={0} />, {
Expand All @@ -95,6 +95,23 @@ describe('Page', () => {
await expect(onLoadSuccessPromise).resolves.toMatchObject([desiredLoadedPage]);
});

it('loads a page and calls onLoadSuccess callback properly when pdf prop is passed', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } = makeAsyncCallback();

render(
<Page
onLoadSuccess={onLoadSuccess}
pageIndex={0}
pdf={pdf}
renderAnnotationLayer={false}
/>,
);

expect.assertions(1);

await expect(onLoadSuccessPromise).resolves.toMatchObject([desiredLoadedPage]);
});

it('returns all desired parameters in onLoadSuccess callback', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } =
makeAsyncCallback<[PageCallback]>();
Expand Down Expand Up @@ -252,7 +269,7 @@ describe('Page', () => {
await expect(onLoadSuccessPromise2).resolves.toMatchObject([desiredLoadedPage2]);
});

it('throws an error when placed outside Document', () => {
it('throws an error when placed outside Document without pdf prop passed', () => {
muteConsole();

expect(() => render(<Page pageIndex={0} />)).toThrow();
Expand Down
10 changes: 4 additions & 6 deletions packages/react-pdf/src/Page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,6 @@ export type PageProps = {
const Page: React.FC<PageProps> = function Page(props) {
const documentContext = useDocumentContext();

invariant(
documentContext,
'Unable to find Document context. Did you wrap <Page /> in <Document />?',
);

const mergedProps = { ...documentContext, ...props };
const {
_className = 'react-pdf__Page',
Expand Down Expand Up @@ -371,7 +366,10 @@ const Page: React.FC<PageProps> = function Page(props) {
const { value: page, error: pageError } = pageState;
const pageElement = useRef<HTMLDivElement>(null);

invariant(pdf, 'Attempted to load a page, but no document was specified.');
invariant(
pdf,
'Attempted to load a page, but no document was specified. Wrap <Page /> in a <Document /> or pass explicit `pdf` prop.',
);

const pageIndex = isProvided(pageNumberProps) ? pageNumberProps - 1 : pageIndexProps ?? null;

Expand Down
13 changes: 6 additions & 7 deletions packages/react-pdf/src/Page/AnnotationLayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ import type { Annotations } from '../shared/types.js';

export default function AnnotationLayer() {
const documentContext = useDocumentContext();

invariant(
documentContext,
'Unable to find Document context. Did you wrap <Page /> in <Document />?',
);

const pageContext = usePageContext();

invariant(pageContext, 'Unable to find Page context.');
Expand All @@ -42,7 +36,12 @@ export default function AnnotationLayer() {
scale = 1,
} = mergedProps;

invariant(
pdf,
'Attempted to load page annotations, but no document was specified. Wrap <Page /> in a <Document /> or pass explicit `pdf` prop.',
);
invariant(page, 'Attempted to load page annotations, but no page was specified.');
invariant(linkService, 'Attempted to load page annotations, but no linkService was specified.');

const [annotationsState, annotationsDispatch] = useResolver<Annotations>();
const { value: annotations, error: annotationsError } = annotationsState;
Expand Down Expand Up @@ -147,7 +146,7 @@ export default function AnnotationLayer() {
);

function renderAnnotationLayer() {
if (!pdf || !page || !annotations) {
if (!pdf || !page || !linkService || !annotations) {
return;
}

Expand Down
14 changes: 12 additions & 2 deletions packages/react-pdf/src/Thumbnail.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('Thumbnail', () => {
});

describe('loading', () => {
it('loads a page and calls onLoadSuccess callback properly', async () => {
it('loads a page and calls onLoadSuccess callback properly when placed inside Document', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } = makeAsyncCallback();

renderWithContext(<Thumbnail onLoadSuccess={onLoadSuccess} pageIndex={0} />, { pdf });
Expand All @@ -78,6 +78,16 @@ describe('Thumbnail', () => {
await expect(onLoadSuccessPromise).resolves.toMatchObject([desiredLoadedThumbnail]);
});

it('loads a page and calls onLoadSuccess callback properly when pdf prop is passed', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } = makeAsyncCallback();

render(<Thumbnail onLoadSuccess={onLoadSuccess} pageIndex={0} pdf={pdf} />);

expect.assertions(1);

await expect(onLoadSuccessPromise).resolves.toMatchObject([desiredLoadedThumbnail]);
});

it('returns all desired parameters in onLoadSuccess callback', async () => {
const { func: onLoadSuccess, promise: onLoadSuccessPromise } =
makeAsyncCallback<[PageCallback]>();
Expand Down Expand Up @@ -190,7 +200,7 @@ describe('Thumbnail', () => {
await expect(onLoadSuccessPromise2).resolves.toMatchObject([desiredLoadedThumbnail2]);
});

it('throws an error when placed outside Document', () => {
it('throws an error when placed outside Document without pdf prop passed', () => {
muteConsole();

expect(() => render(<Thumbnail pageIndex={0} />)).toThrow();
Expand Down
18 changes: 12 additions & 6 deletions packages/react-pdf/src/Thumbnail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,21 @@ export type ThumbnailProps = Omit<
const Thumbnail: React.FC<ThumbnailProps> = function Thumbnail(props) {
const documentContext = useDocumentContext();

invariant(
documentContext,
'Unable to find Document context. Did you wrap <Page /> in <Document />?',
);

const mergedProps = { ...documentContext, ...props };
const {
className,
linkService,
onItemClick,
pageIndex: pageIndexProps,
pageNumber: pageNumberProps,
pdf,
} = mergedProps;

invariant(
pdf,
'Attempted to load a thumbnail, but no document was specified. Wrap <Thumbnail /> in a <Document /> or pass explicit `pdf` prop.',
);

const pageIndex = isProvided(pageNumberProps) ? pageNumberProps - 1 : pageIndexProps ?? null;

const pageNumber = pageNumberProps ?? (isProvided(pageIndexProps) ? pageIndexProps + 1 : null);
Expand All @@ -77,12 +78,17 @@ const Thumbnail: React.FC<ThumbnailProps> = function Thumbnail(props) {
return;
}

invariant(
onItemClick || linkService,
'Either onItemClick callback or linkService must be defined in order to navigate to an outline item.',
);

if (onItemClick) {
onItemClick({
pageIndex,
pageNumber,
});
} else {
} else if (linkService) {
linkService.goToPage(pageNumber);
}
}
Expand Down

0 comments on commit e339525

Please sign in to comment.