Prevent infinite loop while splitting pages
The existing check for whether all children got moved the next page makes sense if we're already partly through the page - but if this happens before we have anything in currentChildren then we'll essentially leave a blank page and move everything to the "next" page, which results in an infinite loop (since the next page never gets smaller).
This PR checks for this situation, and in this case will add the current node to the current page and push the remaining nodes to the next page. This ensures that the next page will get smaller by at least one node so that future iteration won't infinitely loop.
I added a test case that reproduces it - the test doesn't fail it will just hang forever, but if it doesn't loop then the test will pass successfully.
Fixes https://github.com/diegomura/react-pdf/issues/2659
🦋 Changeset detected
Latest commit: 36f1e7511ef9acce2303b5bc31d25382d3b3e92b
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 5 packages
| Name | Type |
|---|---|
| @react-pdf/layout | Patch |
| @react-pdf/renderer | Patch |
| @react-pdf/examples | Patch |
| @react-pdf/e2e-node-cjs | Patch |
| @react-pdf/e2e-node-esm | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
Thanks for this. Unfortunately I have very little time lately and can't test right now. Can someone validate it? 🙏🏻
FWIW I have tested this in my real world scenario (by linking to a locally built copy of react-pdf) that I was running into and it appears to be working properly. @jakeprem did a bunch of investigation in #2659 (huge kudos to him for this btw) - perhaps he'd be able to validate as well.
@diegomura can this be merged/released?
Would be awesome to release because this is probably the reason why our scenario does not work as well and page is crashing all the time
@mantljosh could you help install your version locally and make it as a permanent dependency? I am not strict front-end dev but I hope that your fix could solve lots of issues we have
I am still running into an issue where it's stuck in an infinite loop here. Still investigating, but maybe there could be a hard break after certain runs or something so that it doesn't just hang.
For me, wrapping content of component by empty react tags <>CONTENT</> or some child by it fix all issues with looping, no idea why but try it
Yeah, probably some edge case. For me it was a new section in a report I was generating. Moved the section to a new page to resolve. But the data in the report is dynamic, so it's very possible it could happen again. We definitely need some sort of maximum depth check to break out of the loop and fallback to some default behavior for these cases.
@diegomura Has this been merged in version 4.0.2? I am still running into issued where a marginBottom causes infinite loops. I'd be happy to create a reproducible example.
This is included yes. Please do!
My dynamically created document is a lot more complex, but this simple case crashes the browser if you copy it into the playground at https://react-pdf.org/repl
const Quixote = () => (
<Document>
<Page style={styles.body}>
<View
style={{
border: 1,
height: 700,
marginBottom: 100,
}}
>
<Text style={{ background: "yellow" }}>Test</Text>
</View>
<View
style={{
border: 1,
height: 700,
marginBottom: 100,
}}
>
<Text style={{ background: "yellow" }}>Test</Text>
</View>
</Page>
</Document>
);
@thedivac Your example still let the browser crash. It it the same problem as this? https://github.com/diegomura/react-pdf/issues/2884#issuecomment-2391880139
I don't think it's solved. @thedivac's example still causes a freeze/crash, and so does it in my specific, much more complex case. Has anyone found a solution for it yet?