fix: yoga error and infinite loop while page breaking#3190
fix: yoga error and infinite loop while page breaking#3190diegomura merged 2 commits intodiegomura:masterfrom
Conversation
Fixes diegomura#2974 Returning an undefined width translated into a NaN width when passing the measurement result back to yoga. This NaN width would then be converted to 0 and a warning as follows would be emitted: Measure function returned an invalid dimension to Yoga: [width=nan, height=21.779297] The warnings in yoga's javascript bindings are stored into a long array buffer. If a document is very large (> 100 pages) and contains a lot of text nodes, this can produce millions of yoga warnings, inflating this array buffer, until at some point the browser refuses to push another character onto the end of the buffer. This results in the error `RangeError: Invalid array length at Array.push ()`
🦋 Changeset detectedLatest commit: 3789d74 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 the PR, i can't find this file in |
It is, otherwise I couldn't have created this PR. Please double check you have installed the package correctly according to the documentation. |
When a node is already at the top of the page, it does not help to break it to the next page. We knew that already, but the implementation of this knowledge has been flawed. Apparently, page paddings, page borders, fixed elements and possibly other things can cause the previous position-based detection method to not work correctly. This commit replaces the flawed implementation with a hopefully more solid one: When there are no non-fixed nodes before the current one on the page, we can assume that the current node will not be placed higher up on the page even if we were to break it to the next page. Fixes diegomura#2884 Fixes diegomura#2895 Fixes further parts of diegomura#2974
carlobeltrame
force-pushed
the
fix-invalid-array-length
branch
from
dcb3081 to
3789d74
Compare
carlobeltrame
changed the title
|
Thanks everyone for testing the fixes! |
|
Thanks for the fix @carlobeltrame. I can also confirm that we faced the same problem when generating PDFs greater than 100 pages, and patching the fix, works perfectly. However patching in production is difficult to maintain, and we are waiting for this PR to be merged. |
|
Any plans to get this merged in soon? This error is causing significant issues for my org and would love to get this fix in. @diegomura |
|
@diegomura mind taking a look at this one? Have a few customers that are having issues as a result of this bug. Would appreciate a speedy merge! |
|
pls merge |
|
Fix worked for me. But I am not sure if related, it's only working if I use a standard page size like 'letter', 'a4', etc. The process just hangs if a custom number is set. Using @react-pdf/renderer": "4.3.0 and react 19. |
diegomura
changed the title
@roger-serra-leanon interesting. Can you provide a minimal reproduction example, e.g. using https://react-pdf.org/repl ? |
9 participants
Part 1 of this PR:
Always return both height and width when measuring text
Fixes #2974
Returning an undefined width translated into a NaN width when passing the measurement result back to yoga. This NaN width would then be converted to 0 and a warning as follows would be emitted: Measure function returned an invalid dimension to Yoga: [width=nan, height=21.779297]
The warnings in yoga's javascript bindings are stored into a long array buffer. If a document is very large (> 100 pages) and contains a lot of text nodes, this can produce millions of yoga warnings, inflating this array buffer, until at some point the browser refuses to push another character onto the end of the buffer. This results in the error
RangeError: Invalid array length at Array.push ()Part 2 of this PR:
Try harder to avoid infinite loops while page splitting
Fixes #2884
Fixes #2895
Fixes more issues reported in #2974
When a node is already at the top of the page, it does not help to break it to the next page. We knew that already, but the implementation of this knowledge has been flawed. Apparently, page paddings, page borders, fixed elements and possibly other things can cause the previous position-based detection method to not work correctly.
This commit replaces the flawed implementation with a hopefully more solid one: When there are no non-fixed nodes before the current one on the page, we can assume that the current node will not be placed higher up on the page even if we were to break it to the next page.