react-pdf icon indicating copy to clipboard operation
react-pdf copied to clipboard

Infinite loop hanging megathread (minPresenceAhead, margin, break)

Open tom2strobl opened this issue 11 months ago • 4 comments

Description

  1. There has to be a general flaw in the layout logic resulting in infinite loops that crash browser engines (and whatever the issue might be it is evident that crashes are the worst expression of failure handling)
  2. (maybe related or not) minPresenceAhead works on the simplest examples but fails to work for me in any real-life scenarios where nested Views (eg inevitable for rendering bullet lists) and page padding (necessary for my case) are used

Note that I use 3.3.8 which afaik should already include https://github.com/diegomura/react-pdf/pull/2400

Preliminary work

Regarding 1)

  • https://github.com/diegomura/react-pdf/issues/1384 is related, simply nothing really happened since 2021
  • https://github.com/diegomura/react-pdf/issues/2083 was closed (because OP gone rogue) but seems to be related
  • https://github.com/diegomura/react-pdf/issues/2303 was closed but seems to be related
  • https://github.com/diegomura/react-pdf/issues/2242 might be related
  • https://github.com/diegomura/react-pdf/issues/874 might have been related

Regarding 2)

  • https://github.com/diegomura/react-pdf/issues/2595 absolutely is related and showcases it pretty well
  • https://github.com/diegomura/react-pdf/issues/955 seems it doesn't work for a lot of folks for a very long time

Reproducable cases

(note I'm using @jeetiss REPL for hanging issues since you obviously can't share an url of the official REPL if the process hangs)

1A) Simplest reproducible case REPL: (yes, this is still a thing, nobody confirmed it's fix really)

ReactPDF.render(<Document><Page><Text style={{ paddingBottom: 1000 }}>Text</Text></Page></Document>);

1B) Reducing available space to make it not fit REPL

1C) It's not padding-bound either, as decreasing padding, but increasing font-size has the same effect REPL

1D) In my production app I also have a case (it's a larger document and more complex case that I can't 1:1 easily copy over to a REPL) where marking one of the <View>s with break={true} also suddenly introduces the infinite loop hanging. Changing any wrap or minPresenceAhead props don't affect the outcome, yet changing some margins does (although I was not able to find a real pattern yet unfortunately). So I think it's not the implementation of wrap/minPresenceAhead per se but likely a more general flaw in the layout (splitting?) logic. I'll try to spend some more time somehow getting this into a REPL. My gut says that some certain composition of Views with certain paddings/margins (and/or page padding) is getting too large for the Page and then in the splitting logic it somehow falls into the infinite loop hole.

2A) A more contrived Real-life scenario REPL with the minPresenceAhead issue I removed any fancy logic, but kept a lot of styling on purpose so there's something more elaborate to test again when we're confirming a fix.

minpresence

Thoughts

Interestingly in my hundreds (?) of manual tests I never saw this: https://github.com/diegomura/react-pdf/blob/fab09cc9814326fdb44d2bcb7097ba9960d441d1/packages/layout/src/steps/resolvePagination.js#L44

@carlobeltrame Does https://github.com/diegomura/react-pdf/blob/fab09cc9814326fdb44d2bcb7097ba9960d441d1/packages/layout/src/node/shouldBreak.js#L29 work recursively in a sense that the size and margin/paddings of child Texts of Views of Views are taken into accout? Also – is page size and padding taken into account for the bounding box of the flow? I am asking because of this comment: https://github.com/diegomura/react-pdf/blob/fab09cc9814326fdb44d2bcb7097ba9960d441d1/packages/layout/src/node/shouldBreak.js#L38

What I mean is, if the page has paddingBottom (green) and minPresenceAhead is within the canvas but inside the padding then maybe the following siblings are not taken into account correctly(?):

SCR-20240227-ndmk

I understand that this GH issue might actually be multiple issues at once, but I have the feeling that there must be some general connection.

Desktop (please complete the following information):

  • OS: MacOS
  • Browser: nodejs (electron main thread, but happens in chrome REPL as well)
  • React-pdf version 3.3.8

tom2strobl avatar Feb 27 '24 14:02 tom2strobl