-
-
Notifications
You must be signed in to change notification settings - Fork 901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
React-pdf v7 is more sensitive to rerenders than v6 #1526
Comments
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. |
Check out 7.1.1. Might be a bit better. |
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 |
StrictMode? :) loadDocument is an effect. |
Our app doesn't run in strict mode :L |
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: Screen.Recording.2023-06-15.at.4.04.36.AM.movWith v6, everything works perfectly. For your reference, we're passing a string to the
|
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. |
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 |
Does any of the participants have any updates/findings/breaking examples to share? If not, I'll be closing this in a bit. |
I'm not sure if this is related, but I am getting the error Interestingly, the error only seems to happen when I pass 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. |
@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. |
@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 |
Use useMemo to prevent recreating options object with every render. |
I am trying to solve a virtualisation problem for continious scrolling. The recipe given on the Wiki is not optimal. Tried to use On earlier versions of 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>
); |
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. |
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. |
Do you have an example how to use the useMemo for the . I have the issue it keeps looping.
Using this code, this page kept rerendering:
|
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: #974 #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
The text was updated successfully, but these errors were encountered: