diff --git a/.changeset/lemon-dolphins-hunt.md b/.changeset/lemon-dolphins-hunt.md new file mode 100644 index 000000000..cee4ceab3 --- /dev/null +++ b/.changeset/lemon-dolphins-hunt.md @@ -0,0 +1,6 @@ +--- +"@react-pdf/layout": patch +--- + +Fix yoga error Invalid array length at Array.push() +Fix infinite loop while wrapping pages diff --git a/packages/layout/src/node/shouldBreak.ts b/packages/layout/src/node/shouldBreak.ts index bf164088c..8c0f9dcac 100644 --- a/packages/layout/src/node/shouldBreak.ts +++ b/packages/layout/src/node/shouldBreak.ts @@ -1,5 +1,6 @@ import { SafeNode } from '../types'; import getWrap from './getWrap'; +import isFixed from './isFixed'; const getBreak = (node: SafeNode) => 'break' in node.props ? node.props.break : false; @@ -31,6 +32,7 @@ const shouldBreak = ( child: SafeNode, futureElements: SafeNode[], height: number, + previousElements: SafeNode[], ) => { if ('fixed' in child.props) return false; @@ -42,7 +44,8 @@ const shouldBreak = ( // If the child is already at the top of the page, breaking won't improve its presence // (as long as react-pdf does not support breaking into differently sized containers) - const breakingImprovesPresence = child.box.top > child.box.marginTop; + const breakingImprovesPresence = + previousElements.filter((node: SafeNode) => !isFixed(node)).length > 0; return ( getBreak(child) || diff --git a/packages/layout/src/steps/resolvePagination.ts b/packages/layout/src/steps/resolvePagination.ts index 7d396cb04..f029d7619 100644 --- a/packages/layout/src/steps/resolvePagination.ts +++ b/packages/layout/src/steps/resolvePagination.ts @@ -67,7 +67,12 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => { const nodeTop = getTop(child); const nodeHeight = child.box.height; const isOutside = height <= nodeTop; - const shouldBreak = shouldNodeBreak(child, futureNodes, height); + const shouldBreak = shouldNodeBreak( + child, + futureNodes, + height, + currentChildren, + ); const shouldSplit = height + SAFETY_THRESHOLD < nodeTop + nodeHeight; const canWrap = canNodeWrap(child); const fitsInsidePage = nodeHeight <= contentArea; diff --git a/packages/layout/src/text/measureText.ts b/packages/layout/src/text/measureText.ts index 109a88e3e..ec808c9cf 100644 --- a/packages/layout/src/text/measureText.ts +++ b/packages/layout/src/text/measureText.ts @@ -26,7 +26,7 @@ const measureText = if (widthMode === Yoga.MeasureMode.Exactly) { if (!node.lines) node.lines = layoutText(node, width, height, fontStore); - return { height: linesHeight(node) }; + return { height: linesHeight(node), width }; } if (widthMode === Yoga.MeasureMode.AtMost) { diff --git a/packages/layout/tests/node/shouldBreak.test.ts b/packages/layout/tests/node/shouldBreak.test.ts index e92947ddf..1ef694545 100644 --- a/packages/layout/tests/node/shouldBreak.test.ts +++ b/packages/layout/tests/node/shouldBreak.test.ts @@ -15,6 +15,7 @@ describe('node shouldBreak', () => { }, [], 1000, + [], ); expect(result).toEqual(false); @@ -31,6 +32,7 @@ describe('node shouldBreak', () => { }, [], 1000, + [], ); expect(result).toEqual(true); @@ -54,6 +56,7 @@ describe('node shouldBreak', () => { }, [], 1000, + [], ); expect(result).toEqual(false); @@ -77,6 +80,7 @@ describe('node shouldBreak', () => { }, [], 1000, + [], ); expect(result).toEqual(true); @@ -100,6 +104,7 @@ describe('node shouldBreak', () => { }, [], 1000, + [], ); expect(result).toEqual(true); @@ -142,6 +147,24 @@ describe('node shouldBreak', () => { }, ], 1000, + [ + { + type: 'VIEW', + props: {}, + style: {}, + children: [], + box: { + top: 900, + right: 0, + bottom: 0, + left: 0, + height: 200, + width: 200, + marginTop: 0, + marginBottom: 0, + }, + }, + ], ); expect(result).toEqual(true); @@ -184,6 +207,24 @@ describe('node shouldBreak', () => { }, ], 1000, + [ + { + type: 'VIEW', + props: {}, + style: {}, + children: [], + box: { + top: 900, + right: 0, + bottom: 0, + left: 0, + height: 200, + width: 200, + marginTop: 0, + marginBottom: 0, + }, + }, + ], ); expect(result).toEqual(true); @@ -226,6 +267,7 @@ describe('node shouldBreak', () => { }, ], 1000, + [], ); expect(result).toEqual(false); @@ -268,6 +310,7 @@ describe('node shouldBreak', () => { }, ], 1000, + [], ); expect(result).toEqual(false); @@ -310,6 +353,7 @@ describe('node shouldBreak', () => { }, ], 1000, + [], ); expect(result).toEqual(false); @@ -352,12 +396,56 @@ describe('node shouldBreak', () => { }, ], 1000, + [], + ); + + expect(result).toEqual(false); + }); + + 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', () => { + const result = shouldBreak( + { + type: 'VIEW', + props: { minPresenceAhead: 400 }, + style: {}, + children: [], + box: { + top: 500, + right: 0, + bottom: 0, + left: 0, + height: 400, + width: 200, + marginTop: 500, + marginBottom: 0, + }, + }, + [ + { + type: 'VIEW', + props: {}, + style: {}, + children: [], + box: { + top: 900, + right: 0, + bottom: 0, + left: 0, + height: 200, + width: 200, + marginTop: 0, + marginBottom: 0, + }, + }, + ], + 1000, + [], ); expect(result).toEqual(false); }); - test('should not break due to minPresenceAhead when breaking does not improve presence, to avoid infinite loops', () => { + test('should not break due to minPresenceAhead even when there are some previous fixed nodes on the page, to avoid infinite loops', () => { const result = shouldBreak( { type: 'VIEW', @@ -394,6 +482,26 @@ describe('node shouldBreak', () => { }, ], 1000, + [ + { + type: 'VIEW', + props: { + fixed: true, + }, + style: {}, + children: [], + box: { + top: 900, + right: 0, + bottom: 0, + left: 0, + height: 200, + width: 200, + marginTop: 0, + marginBottom: 0, + }, + }, + ], ); expect(result).toEqual(false); @@ -436,6 +544,7 @@ describe('node shouldBreak', () => { }, ], 1000, + [], ); expect(result).toEqual(false); @@ -478,6 +587,7 @@ describe('node shouldBreak', () => { }, ], 1000, + [], ); expect(result).toEqual(false); @@ -536,6 +646,7 @@ describe('node shouldBreak', () => { }, ], 811.89, + [], ); expect(result).toEqual(false); @@ -594,6 +705,7 @@ describe('node shouldBreak', () => { }, ], 811.89, + [], ); expect(result).toEqual(false); @@ -721,6 +833,7 @@ describe('node shouldBreak', () => { }, ], 781.89, + [], ); expect(result).toEqual(false); @@ -763,6 +876,7 @@ describe('node shouldBreak', () => { }, ], 776.89, + [], ); expect(result).toEqual(false); diff --git a/packages/layout/tests/steps/resolvePagination.test.ts b/packages/layout/tests/steps/resolvePagination.test.ts index 9cf55bdf9..f8ca257b3 100644 --- a/packages/layout/tests/steps/resolvePagination.test.ts +++ b/packages/layout/tests/steps/resolvePagination.test.ts @@ -276,4 +276,67 @@ describe('pagination step', () => { // If calcLayout returns then we did not hit an infinite loop expect(true).toBe(true); }); + + test('should take padding into account when splitting pages', async () => { + const yoga = await loadYoga(); + + const root = { + type: 'DOCUMENT' as const, + yoga, + props: {}, + style: {}, + children: [ + { + type: 'PAGE' as const, + box: { + width: 612, + height: 792, + top: 0, + left: 0, + right: 612, + bottom: 792, + }, + style: { + paddingTop: 30, + width: 612, + height: 792, + }, + props: { wrap: true }, + children: [ + { + type: 'VIEW' as const, + box: { + width: 612, + height: 761, + top: 0, + left: 0, + right: 612, + bottom: 761, + }, + style: { height: 761, marginBottom: 24 }, + props: { wrap: true, break: false }, + }, + { + type: 'VIEW' as const, + box: { + width: 612, + height: 80, + top: 761, + left: 0, + right: 612, + bottom: 841, + }, + style: { height: 80 }, + props: { wrap: true, break: false }, + }, + ], + }, + ], + }; + + calcLayout(root); + + // If calcLayout returns then we did not hit an infinite loop + expect(true).toBe(true); + }); }); diff --git a/packages/layout/tests/text/measureText.test.ts b/packages/layout/tests/text/measureText.test.ts new file mode 100644 index 000000000..3e757d940 --- /dev/null +++ b/packages/layout/tests/text/measureText.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from 'vitest'; + +import * as P from '@react-pdf/primitives'; +import FontStore from '@react-pdf/font'; +import { SafeTextNode } from '../../src'; +import measureText from '../../src/text/measureText'; + +const TEXT = + 'Life can be much broader once you discover one simple fact: Everything around you that you call life was made up by people that were no smarter than you'; + +const fontStore = new FontStore(); + +const createTextNode = ( + value: string, + style = {}, + props = {}, +): SafeTextNode => ({ + style, + props, + type: P.Text, + children: [{ type: P.TextInstance, value }], +}); + +describe('measureText', () => { + describe('widthMode Exactly', () => { + test('should return width and height', async () => { + const page = { + type: 'PAGE' as const, + props: {}, + style: {}, + }; + const node = createTextNode(TEXT); + const measureFunc = measureText(page, node, fontStore); + + const dimensions = measureFunc(100, 1 /*Yoga.MeasureMode.Exactly*/, 50); + + expect(dimensions.width).toStrictEqual(100); + expect(dimensions.height).toBe(39.599999999999994); + }); + }); + + describe('widthMode AtMost', () => { + test('should return width and height', async () => { + const page = { + type: 'PAGE' as const, + props: {}, + style: {}, + }; + const node = createTextNode(TEXT); + const measureFunc = measureText(page, node, fontStore); + + const dimensions = measureFunc(100, 2 /*Yoga.MeasureMode.AtMost*/, 50); + + expect(dimensions.width).toStrictEqual(100); + expect(dimensions.height).toBe(39.599999999999994); + }); + }); +});