Skip to content

Commit a99d10f

Browse files
carlobeltramecarlobeltrame
authored
fix: yoga error and infinite loop while page breaking (#3190)
1 parent ee5c96b commit a99d10f

7 files changed

Lines changed: 253 additions & 4 deletions

File tree

‎.changeset/lemon-dolphins-hunt.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@react-pdf/layout": patch
3+
---
4+
5+
Fix yoga error Invalid array length at Array.push(<anonymous>)
6+
Fix infinite loop while wrapping pages

‎packages/layout/src/node/shouldBreak.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { SafeNode } from '../types';
22
import getWrap from './getWrap';
3+
import isFixed from './isFixed';
34

45
const getBreak = (node: SafeNode) =>
56
'break' in node.props ? node.props.break : false;
@@ -31,6 +32,7 @@ const shouldBreak = (
3132
child: SafeNode,
3233
futureElements: SafeNode[],
3334
height: number,
35+
previousElements: SafeNode[],
3436
) => {
3537
if ('fixed' in child.props) return false;
3638

@@ -42,7 +44,8 @@ const shouldBreak = (
4244

4345
// If the child is already at the top of the page, breaking won't improve its presence
4446
// (as long as react-pdf does not support breaking into differently sized containers)
45-
const breakingImprovesPresence = child.box.top > child.box.marginTop;
47+
const breakingImprovesPresence =
48+
previousElements.filter((node: SafeNode) => !isFixed(node)).length > 0;
4649

4750
return (
4851
getBreak(child) ||

‎packages/layout/src/steps/resolvePagination.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,12 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
6767
const nodeTop = getTop(child);
6868
const nodeHeight = child.box.height;
6969
const isOutside = height <= nodeTop;
70-
const shouldBreak = shouldNodeBreak(child, futureNodes, height);
70+
const shouldBreak = shouldNodeBreak(
71+
child,
72+
futureNodes,
73+
height,
74+
currentChildren,
75+
);
7176
const shouldSplit = height + SAFETY_THRESHOLD < nodeTop + nodeHeight;
7277
const canWrap = canNodeWrap(child);
7378
const fitsInsidePage = nodeHeight <= contentArea;

‎packages/layout/src/text/measureText.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const measureText =
2626
if (widthMode === Yoga.MeasureMode.Exactly) {
2727
if (!node.lines) node.lines = layoutText(node, width, height, fontStore);
2828

29-
return { height: linesHeight(node) };
29+
return { height: linesHeight(node), width };
3030
}
3131

3232
if (widthMode === Yoga.MeasureMode.AtMost) {

‎packages/layout/tests/node/shouldBreak.test.ts‎

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe('node shouldBreak', () => {
1515
},
1616
[],
1717
1000,
18+
[],
1819
);
1920

2021
expect(result).toEqual(false);
@@ -31,6 +32,7 @@ describe('node shouldBreak', () => {
3132
},
3233
[],
3334
1000,
35+
[],
3436
);
3537

3638
expect(result).toEqual(true);
@@ -54,6 +56,7 @@ describe('node shouldBreak', () => {
5456
},
5557
[],
5658
1000,
59+
[],
5760
);
5861

5962
expect(result).toEqual(false);
@@ -77,6 +80,7 @@ describe('node shouldBreak', () => {
7780
},
7881
[],
7982
1000,
83+
[],
8084
);
8185

8286
expect(result).toEqual(true);
@@ -100,6 +104,7 @@ describe('node shouldBreak', () => {
100104
},
101105
[],
102106
1000,
107+
[],
103108
);
104109

105110
expect(result).toEqual(true);
@@ -142,6 +147,24 @@ describe('node shouldBreak', () => {
142147
},
143148
],
144149
1000,
150+
[
151+
{
152+
type: 'VIEW',
153+
props: {},
154+
style: {},
155+
children: [],
156+
box: {
157+
top: 900,
158+
right: 0,
159+
bottom: 0,
160+
left: 0,
161+
height: 200,
162+
width: 200,
163+
marginTop: 0,
164+
marginBottom: 0,
165+
},
166+
},
167+
],
145168
);
146169

147170
expect(result).toEqual(true);
@@ -184,6 +207,24 @@ describe('node shouldBreak', () => {
184207
},
185208
],
186209
1000,
210+
[
211+
{
212+
type: 'VIEW',
213+
props: {},
214+
style: {},
215+
children: [],
216+
box: {
217+
top: 900,
218+
right: 0,
219+
bottom: 0,
220+
left: 0,
221+
height: 200,
222+
width: 200,
223+
marginTop: 0,
224+
marginBottom: 0,
225+
},
226+
},
227+
],
187228
);
188229

189230
expect(result).toEqual(true);
@@ -226,6 +267,7 @@ describe('node shouldBreak', () => {
226267
},
227268
],
228269
1000,
270+
[],
229271
);
230272

231273
expect(result).toEqual(false);
@@ -268,6 +310,7 @@ describe('node shouldBreak', () => {
268310
},
269311
],
270312
1000,
313+
[],
271314
);
272315

273316
expect(result).toEqual(false);
@@ -310,6 +353,7 @@ describe('node shouldBreak', () => {
310353
},
311354
],
312355
1000,
356+
[],
313357
);
314358

315359
expect(result).toEqual(false);
@@ -352,12 +396,56 @@ describe('node shouldBreak', () => {
352396
},
353397
],
354398
1000,
399+
[],
400+
);
401+
402+
expect(result).toEqual(false);
403+
});
404+
405+
test('should not break due to minPresenceAhead when breaking does not improve presence because the node is already the first non-fixed node on the page, to avoid infinite loops', () => {
406+
const result = shouldBreak(
407+
{
408+
type: 'VIEW',
409+
props: { minPresenceAhead: 400 },
410+
style: {},
411+
children: [],
412+
box: {
413+
top: 500,
414+
right: 0,
415+
bottom: 0,
416+
left: 0,
417+
height: 400,
418+
width: 200,
419+
marginTop: 500,
420+
marginBottom: 0,
421+
},
422+
},
423+
[
424+
{
425+
type: 'VIEW',
426+
props: {},
427+
style: {},
428+
children: [],
429+
box: {
430+
top: 900,
431+
right: 0,
432+
bottom: 0,
433+
left: 0,
434+
height: 200,
435+
width: 200,
436+
marginTop: 0,
437+
marginBottom: 0,
438+
},
439+
},
440+
],
441+
1000,
442+
[],
355443
);
356444

357445
expect(result).toEqual(false);
358446
});
359447

360-
test('should not break due to minPresenceAhead when breaking does not improve presence, to avoid infinite loops', () => {
448+
test('should not break due to minPresenceAhead even when there are some previous fixed nodes on the page, to avoid infinite loops', () => {
361449
const result = shouldBreak(
362450
{
363451
type: 'VIEW',
@@ -394,6 +482,26 @@ describe('node shouldBreak', () => {
394482
},
395483
],
396484
1000,
485+
[
486+
{
487+
type: 'VIEW',
488+
props: {
489+
fixed: true,
490+
},
491+
style: {},
492+
children: [],
493+
box: {
494+
top: 900,
495+
right: 0,
496+
bottom: 0,
497+
left: 0,
498+
height: 200,
499+
width: 200,
500+
marginTop: 0,
501+
marginBottom: 0,
502+
},
503+
},
504+
],
397505
);
398506

399507
expect(result).toEqual(false);
@@ -436,6 +544,7 @@ describe('node shouldBreak', () => {
436544
},
437545
],
438546
1000,
547+
[],
439548
);
440549

441550
expect(result).toEqual(false);
@@ -478,6 +587,7 @@ describe('node shouldBreak', () => {
478587
},
479588
],
480589
1000,
590+
[],
481591
);
482592

483593
expect(result).toEqual(false);
@@ -536,6 +646,7 @@ describe('node shouldBreak', () => {
536646
},
537647
],
538648
811.89,
649+
[],
539650
);
540651

541652
expect(result).toEqual(false);
@@ -594,6 +705,7 @@ describe('node shouldBreak', () => {
594705
},
595706
],
596707
811.89,
708+
[],
597709
);
598710

599711
expect(result).toEqual(false);
@@ -721,6 +833,7 @@ describe('node shouldBreak', () => {
721833
},
722834
],
723835
781.89,
836+
[],
724837
);
725838

726839
expect(result).toEqual(false);
@@ -763,6 +876,7 @@ describe('node shouldBreak', () => {
763876
},
764877
],
765878
776.89,
879+
[],
766880
);
767881

768882
expect(result).toEqual(false);

‎packages/layout/tests/steps/resolvePagination.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,4 +276,67 @@ describe('pagination step', () => {
276276
// If calcLayout returns then we did not hit an infinite loop
277277
expect(true).toBe(true);
278278
});
279+
280+
test('should take padding into account when splitting pages', async () => {
281+
const yoga = await loadYoga();
282+
283+
const root = {
284+
type: 'DOCUMENT' as const,
285+
yoga,
286+
props: {},
287+
style: {},
288+
children: [
289+
{
290+
type: 'PAGE' as const,
291+
box: {
292+
width: 612,
293+
height: 792,
294+
top: 0,
295+
left: 0,
296+
right: 612,
297+
bottom: 792,
298+
},
299+
style: {
300+
paddingTop: 30,
301+
width: 612,
302+
height: 792,
303+
},
304+
props: { wrap: true },
305+
children: [
306+
{
307+
type: 'VIEW' as const,
308+
box: {
309+
width: 612,
310+
height: 761,
311+
top: 0,
312+
left: 0,
313+
right: 612,
314+
bottom: 761,
315+
},
316+
style: { height: 761, marginBottom: 24 },
317+
props: { wrap: true, break: false },
318+
},
319+
{
320+
type: 'VIEW' as const,
321+
box: {
322+
width: 612,
323+
height: 80,
324+
top: 761,
325+
left: 0,
326+
right: 612,
327+
bottom: 841,
328+
},
329+
style: { height: 80 },
330+
props: { wrap: true, break: false },
331+
},
332+
],
333+
},
334+
],
335+
};
336+
337+
calcLayout(root);
338+
339+
// If calcLayout returns then we did not hit an infinite loop
340+
expect(true).toBe(true);
341+
});
279342
});

0 commit comments

Comments
 (0)