wojtekmaj / react-pdf

Display PDFs in your React app as easily as if they were images.
https://projects.wojtekmaj.pl/react-pdf
MIT License
9.28k stars 878 forks source link

React-pdf v7 is more sensitive to rerenders than v6 #1526

Closed MattL75 closed 9 months ago

MattL75 commented 1 year ago

Before you start - checklist

Description

Our app was working fine in v6. After migrating to v7, we started seeing many bugs. In some cases, infinite loops of loading workers and documents. We have a fairly complex and convoluted setup (legacy app) so I am having a very hard time reproducing. The errors we see after a little bit are the ones in the below tickets, but I don't think the tickets are related. The info seems very different.

Related issues: https://github.com/wojtekmaj/react-pdf/issues/974 https://github.com/wojtekmaj/react-pdf/issues/1062

Here's the kicker. If I replace the Document.tsx component with the Document.jsx component from v6, everything works fine.

I'm assuming that turning the components to hooks made them more sensitive. In particular, I'm theorizing that it's related to https://github.com/wojtekmaj/react-pdf/blob/v6.2.2/src/Document.jsx#L78 as with the hook implementation there is really nothing preventing a rerender of the same file if you end up in a weird async state (which we are).

Just opened the ticket to maybe get some opinions and have a place for others to maybe share some ideas.

Steps to reproduce

Currently working on it. Been trying to hack together a repro for a few days and no luck.

Expected behavior

No infinite loops

Actual behavior

Infinite loops

Additional information

@wojtekmaj I don't have a repro yet. I can just tell you that replacing the v7 Document.tsx with the Document.jsx implementation in v6 fixes the problem.

I thought I'd still create a ticket to pool other people having the issue, if any.

Environment

vsolominov commented 1 year ago

Ran into a similar problem when upgrading react-pdf from version 6.2.2 to version 7+. I use react-virtuoso to work with large multi-page documents. On version 6.2.2, I did not notice any problems. After updating to 7.1.0, I began to observe problems when scrolling (especially if you scroll quickly up). I suspect that this is due to the constant re-rendering of pages. Wrote a simple example to illustrate the problem.

wojtekmaj commented 1 year ago

Check out 7.1.1. Might be a bit better.

MattL75 commented 1 year ago

Seems to not address the issue though I can see there are a couple fewer renders which is good.

The key seems to be that loadDocument is called more frequently, at least this is the issue I see in the debugger.

Previously, if the children nodes needed to rerender, Document did not necessarily call loadDocument. Now it does, which I don't think is the desired outcome. Still trying to find a way to represent the issue in codesandbox

wojtekmaj commented 1 year ago

StrictMode? :) loadDocument is an effect.

MattL75 commented 1 year ago

Our app doesn't run in strict mode :L

allroundexperts commented 1 year ago

This seems to be a real issue with our App as well, making us unable to use v7. Here's the behaviour that we're encountering:

https://github.com/wojtekmaj/react-pdf/assets/30054992/da01a13f-6e0e-4ba8-8362-992f641eed39

With v6, everything works perfectly.

For your reference, we're passing a string to the Document component's file prop. The value we are passing currently is:

http://localhost:8080/staging/chat-attachments/2918182810622930510/w_d48c42570a46b949c970f3f2a574a9b4301dd2a0.pdf?encryptedAuthToken=Qs%2B%2BLL2X7%2F4suOx9LAnEDYQDkH3AeexkTEJSt0QdjzpcOvz9%2BvQTUK9e5QWdUFJ3YrDI5K8V9VFXitCecQhShwOIS1ni7P0hXoiikK6UYTb5QmBzbwYfl%2FLvfZvP5ZZ455D%2FCqlqZfKV1W8iMuQP9d2z8MrXjxQved2aEt5I49AVShbWO246lBNRAm2fdK%2FDFitLSmiExmPJW3HdiAhS7lKWwejmneAGr4AR8klvLn5dKuFvgWfuMcKfT5xcrzL8cdro3s%2BrlrsVS8ltiSjB6604BmnbgEmDTBbhwR3%2Bt5W5B5ijr7khpqLZ1ZGCpu664V%2BWhif2iBtrgh7S6RmLNTsNyBAHqacosv5MlFr%2BVc3aknIQyf1Y0wqNExfAMKJxmQkOEutkx7pwR1FhrrAw0kk5j4fUPMxfZWAJ3bKNNTdXv0N3jEIz00jYXu%2FWSK%2FVUxs5t84PBDO1yu%2Fjwms%2FMSXQhj5ZSkIEP4aWwiWgSudhOSflbYULV0nCywBzyG%2FvZAavs5q5JK0lIbnenBCis8XKR7OZOFsd2Hlpti4xx%2FXEPIthX8Nt5Sw4WcFmXIvKoGQUL3UNiJBFj%2FVZX0FJ%2FsayUPXL2r5gMbkb74TzrXGWKco%2FlggvIpJjKXRGBSNIDyF75nCZvCMtXA0AfyIvBwrOypeT2DDJ%2FG19bO%2B8paFsu5lmsBTr13L66DlY%2FIzJdEwrnkQxNeBDMcCS22ilPOKlGGTKIUeblKYXEOV%2FLeRqNNdurl%2BPVDKZwwx4LxqzQxJXrGR71cxiHR2dATPixocL1Q1cQlzEwwnKd9vgS2VDcx37wqAV5n%2F%2BIvpjn9HE2hZUF8hzO7BawPdywf0AS8bU0j10ab6T%2Fw6d9sbDauSWCY8Tum6M9qLRJA0kFgFHQaakWjXB2kut%2Frk%2BnS0bR3lFlyI5pDb%2FG0X6DzzzGGZ4kUGoTKq5RXaKMDQKdS%2Bdb%2FH785YWgtfu2wJ8N0NxktPE9PYxNkUCwjYObD6Ih%2F%2FBT3WirA7AIV8dzW3ulVlW
Hideman85 commented 1 year ago

We cannot work either with v7, it does not render in the canvas at all, the text layer is here, there is no error, nothing, downgrading to 6.2.2 works perfectly. It is any pdf we used in the app, none render with v7 and no error is thrown apparently.

Evanc123 commented 1 year ago

I've also had to roll back to 6.2.2, v7 the text layer causes rendering issues. will post more details in a bit

wojtekmaj commented 12 months ago

Does any of the participants have any updates/findings/breaking examples to share? If not, I'll be closing this in a bit.

vstollen commented 11 months ago

I'm not sure if this is related, but I am getting the error this.messageHandler is null in my implementation. The error does not occur when I downgrade to 6.2.2.

Interestingly, the error only seems to happen when I pass options to <Document>.

I have added a minimal reproduction here: https://codesandbox.io/s/react-pdf-7-3-3-errors-t3mfc2

In our actual implementation, we debounce the resize events and have a little more logic involved, which I have excluded for simplicity.

wojtekmaj commented 11 months ago

@vstollen This has to do with the way you've defined options. Documentation suggests doing so outside of React function and for a good reason. Once I did this, the error was gone.

vstollen commented 11 months ago

@wojtekmaj thanks for the prompt response!

I do not want to pollute this issue with unrelated problems, but would it be possible to have a dynamic options object?

Specifically, I want to add an Authorization header in my options object, where the token is a reactive value that changes occasionally.

wojtekmaj commented 11 months ago

Use useMemo to prevent recreating options object with every render.

vsolominov commented 11 months ago

Does any of the participants have any updates/findings/breaking examples to share? If not, I'll be closing this in a bit.

I am trying to solve a virtualisation problem for continious scrolling. The recipe given on the Wiki is not optimal.

Tried to use react-window as a third party library. Found your example react-pdf-react-window-fullscreen. However, page rendering is quite slow with this library.

On earlier versions of reac-pdf I used the react-virtuoso library, which had good performance. However, since versions 7+, page rendering is "chaotic" and "flies off into space" with a little scrolling. Sample code:

const pageCount = 14;
const pageHeight = 700;
const rootElement = document.getElementById("app");

createRoot(rootElement).render(
  <StrictMode>
    <Document key={"Test.pdf"} file={samplePDF}>
      <Virtuoso
        style={{ height: `${pageHeight}px` }}
        totalCount={pageCount}
        defaultItemHeight={pageHeight}
        itemContent={(index) => <Page pageIndex={index} height={pageHeight} />}
      />
    </Document>
  </StrictMode>
);

example

pmcb99 commented 10 months ago

@vstollen This has to do with the way you've defined options. Documentation suggests doing so outside of React function and for a good reason. Once I did this, the error was gone.

I found that doing this outside the React function does not prevent the error on v7.3.3. When I remove options, the error disappears.

MattL75 commented 9 months ago

Forgot to report back here. Fixed this a while back. Our app had dynamic options property. Took quite a bit of refactoring but we managed to remove that need. Still not sure why it was working in v6 but not in v7. Anyways, for anybody having this issue, just define static options either outside the react component or in a useMemo.

I see a lot of unrelated issues in this thread. If you have an issue different from this one please make a new ticket with your own circumstances so it can be tracked better. For now I will close this one.

jovana commented 8 months ago

Use useMemo to prevent recreating options object with every render.

Do you have an example how to use the useMemo for the . I have the issue it keeps looping. Using this code it works, and gives me one page:

<Page pageNumber={1} />

Using this code, this page kept rerendering:


 <Document file={file} onLoadSuccess={onDocumentLoadSuccess} options={options}>
            {Array.from(new Array(numPages), (el, index) => (
              <Page
                key={`page_${index + 1}`}
                pageNumber={index + 1}
                width={containerWidth ? Math.min(containerWidth, maxWidth) : maxWidth}
              />
            ))}
          </Document>
``` (this code snipped is from the example page: https://github.com/wojtekmaj/react-pdf/blob/0cbb75af635affdb1c0a8027c94e3fda509f0291/sample/create-react-app-5/src/Sample.tsx#L66) This problem seems in the versions 7.1.1 and 7.4 and 7.6.

Thanks for helping me out!!