fix(layout): don't squash a container's sole child at a page break (#3449)

.changeset/pagination-page-break-overlap.md

@@ -0,0 +1,5 @@
+---
+'@react-pdf/layout': patch
+---
+
+Fix content overlapping itself at a page break. A node that is the sole child of its container and lands near the bottom of a page (e.g. a `wrap={false}` block, or an element taller than the space left) was force-fit into the remaining space instead of moving to the next page, drawing its lines on top of each other. Pagination now bases that decision on whether the page is actually empty above the node, which also avoids an infinite page loop. Closes #3449.

REPL.md

@@ -0,0 +1,140 @@
+# Reproducing #3449 — content overlapping itself at a page break
+
+This document shows how to reproduce, by hand, the pagination bug fixed in this
+branch: a block that lands near the bottom of a page is **squashed into the few
+points of space left** — its lines drawn on top of each other — instead of
+moving cleanly to the next page.
+
+- **Affected package:** `@react-pdf/layout` (the pagination step). It is what
+  `@react-pdf/renderer` uses to break a document into pages, so the bug shows up
+  through the renderer even though the renderer version number looks unrelated
+  (e.g. renderer `4.5.1` depends on layout `4.6.1`).
+- **Still present on the latest published version** (layout `4.6.1`).
+
+## Quick reproduction (react-pdf REPL)
+
+1. Open <https://react-pdf.org/repl>.
+2. Replace the editor contents with the snippet below.
+3. Look at the **bottom of page 1**.
+
+```jsx
+const Block = ({ label, value }) => (
+  <View style={{ marginTop: 5 }} wrap={false}>
+    <Text style={{ fontSize: 8, fontWeight: 700 }}>{label}</Text>
+    <Text style={{ fontSize: 8 }}>{value}</Text>
+  </View>
+);
+
+const Entry = ({ time, title, a, w, p }) => (
+  <View>
+    <View style={{ flexDirection: "row", borderBottomWidth: 1, paddingVertical: 8 }} wrap={false}>
+      <View style={{ width: 130, marginLeft: 20 }}>
+        <Text style={{ fontSize: 11, marginBottom: 5 }}>{time}</Text>
+        <Text style={{ fontSize: 7 }}>PENDING</Text>
+      </View>
+      <View style={{ flex: 1, marginRight: 20 }}>
+        <Text style={{ fontSize: 10 }}>{title}</Text>
+        <Text style={{ fontSize: 8, marginTop: 5 }}>2 Adults</Text>
+        <Block label="Address" value={a} />
+        <Block label="Website" value={w} />
+        <Block label="Phone" value={p} />
+      </View>
+    </View>
+  </View>
+);
+
+const Doc = () => (
+  <Document>
+    <Page size="A4" style={{ paddingTop: 40, paddingBottom: 90, paddingHorizontal: 40, flexDirection: "column" }}>
+      <View fixed>
+        <Text style={{ fontSize: 22 }}>Daily Itinerary</Text>
+      </View>
+      <View style={{ flex: 1 }}>
+        {/* spacer: pushes the 2nd entry to the bottom of page 1 */}
+        <View style={{ height: 500 }} />
+        <View>
+          <View minPresenceAhead={100}>
+            <View style={{ marginTop: 10, marginBottom: 8 }}>
+              <Text style={{ fontSize: 9, fontWeight: 700 }}>TUESDAY, APR 7</Text>
+            </View>
+            <Entry time="12:25 AM" title="Juvia"
+              a="1111 Lincoln Road, Miami Beach, Florida, 33139"
+              w="http://www.juviamiami.com/" p="+1 305-763-8272" />
+          </View>
+          <View>
+            <Entry time="01:05 AM" title="Miami Beach Golf Club"
+              a="2301 Alton Road, Miami Beach, Florida, 33140"
+              w="http://www.miamibeachgolfclub.com/" p="+1 305-532-3350" />
+          </View>
+        </View>
+      </View>
+      <View style={{ position: "absolute", bottom: 0, left: 40, right: 40, height: 50 }} fixed>
+        <Text style={{ fontSize: 8 }}>EXCLUSIVE RESORTS</Text>
+      </View>
+    </Page>
+  </Document>
+);
+
+ReactPDF.render(<Doc />);
+```
+
+### What you should see (the bug)
+
+At the bottom of page 1, the **"01:05 AM / Miami Beach Golf Club"** entry is
+crammed into the sliver of space left:
+
+- `Address` is drawn on top of `2301 Alton Road, …`
+- `Website` is drawn on top of the URL
+- `Phone` is drawn on top of the phone number
+
+i.e. every field of that entry overlaps the next. With the fix, the entry moves
+to page 2 intact and page 1 ends cleanly after the "Juvia" entry.
+
+### Important: the trigger is position-sensitive
+
+The overlap only happens when the second entry lands with **just barely not
+enough** room — enough to *start* on the page but not enough to fit. The exact
+`height` that lands it there depends on font metrics, which differ slightly
+between the in-browser REPL and a server render. The verified window is roughly
+**`height: 490`–`510`**. If `500` renders cleanly for you, try `490` or `510`.
+
+## Why it happens (root cause)
+
+In `packages/layout/src/steps/resolvePagination.ts`, `splitNodes` decides what
+to do when a node is split and **all** of its children move to the next page
+(leaving an empty shell on the current page):
+
+```js
+if (child.children.length > 0 && currentChild.children.length === 0) {
+  // "if the current page is empty, keep the parent here"
+  if (currentChildren.length === 0) {
+    currentChildren.push(child, ...futureFixedNodes); // kept on the current page
+  } else {
+    // move it to the next page
+  }
+}
+```
+
+The intent of `currentChildren.length === 0` is *"the page is empty, so keeping
+this node here (even if it overflows) is better than producing an empty page /
+looping forever."* But `currentChildren` is the **local** list for the node's
+own container — it is `0` simply because the node is the **first child of its
+container**, which is not the same as the page being empty.
+
+In the snippet above the golf entry is the only child of its (sibling)
+container, yet the page already holds the "Juvia" entry. The check misfires,
+the whole entry is force-kept in the tiny remaining space, and Yoga compresses
+its lines on top of each other.
+
+## The fix
+
+Base the keep-vs-move decision on whether the page is actually **empty above
+the node**, threaded through the split recursion (`pageEmpty`):
+
+- Page already has content above → **move** the node to the next page.
+- Page genuinely empty and the node cannot fit → keep it (and let it overflow),
+  which is what preserves termination and prevents an infinite page loop.
+
+See `packages/layout/src/steps/resolvePagination.ts` and the regression tests in
+`packages/layout/tests/steps/resolvePagination.test.ts` (overlap sweep across
+page heights + a tall-fixed-header loop guard).

packages/layout/src/steps/resolvePagination.ts

@@ -55,7 +55,16 @@ const warnUnavailableSpace = (node: SafeNode) => {
   );
 };
 
-const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
+const splitNodes = (
+  height: number,
+  contentArea: number,
+  nodes: SafeNode[],
+  // Whether the current page has no laid-out (non-fixed) content above these
+  // nodes yet. Threaded through the recursion so the "keep an un-splittable
+  // node on the current page" decision reflects the *page*, not just the local
+  // container. See https://github.com/diegomura/react-pdf/issues/3449
+  pageEmpty = true,
+) => {
   const currentChildren: SafeNode[] = [];
   const nextChildren: SafeNode[] = [];
 
@@ -64,6 +73,11 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
     const futureNodes = nodes.slice(i + 1);
     const futureFixedNodes = futureNodes.filter(isFixed);
 
+    // The page is empty above this node only when it was empty entering this
+    // container and nothing non-fixed has been placed before it here.
+    const emptyAbove =
+      pageEmpty && !currentChildren.some((node) => !isFixed(node));
+
     const nodeTop = getTop(child);
     const nodeHeight = child.box.height;
     const isOutside = height <= nodeTop;
@@ -111,12 +125,24 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
     }
 
     if (shouldSplit) {
-      const [currentChild, nextChild] = split(child, height, contentArea);
+      const [currentChild, nextChild] = split(
+        child,
+        height,
+        contentArea,
+        emptyAbove,
+      );
 
       // All children are moved to the next page, it doesn't make sense to show the parent on the current page
       if (child.children.length > 0 && currentChild.children.length === 0) {
-        // But if the current page is empty then we can just include the parent on the current page
-        if (currentChildren.length === 0) {
+        // Keep the (now empty) parent on the current page only when the page is
+        // empty above it — otherwise there is nothing to gain and we would just
+        // squash an oversized subtree into the little space left here, drawing
+        // its lines on top of each other. When the page already has content, we
+        // move the whole node to the next page instead. Using page emptiness
+        // (not the local `currentChildren.length === 0`) is what prevents both
+        // the overlap and an infinite page loop.
+        // See https://github.com/diegomura/react-pdf/issues/3449
+        if (emptyAbove) {
           currentChildren.push(child, ...futureFixedNodes);
           nextChildren.push(...futureNodes);
         } else {
@@ -143,18 +169,29 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
   return [currentChildren, nextChildren];
 };
 
-const splitChildren = (height: number, contentArea: number, node: SafeNode) => {
+const splitChildren = (
+  height: number,
+  contentArea: number,
+  node: SafeNode,
+  pageEmpty: boolean,
+) => {
   const children = node.children || [];
   const availableHeight = height - getTop(node);
-  return splitNodes(availableHeight, contentArea, children);
+  return splitNodes(availableHeight, contentArea, children, pageEmpty);
 };
 
-const splitView = (node: SafeNode, height: number, contentArea: number) => {
+const splitView = (
+  node: SafeNode,
+  height: number,
+  contentArea: number,
+  pageEmpty: boolean,
+) => {
   const [currentNode, nextNode] = splitNode(node, height);
   const [currentChilds, nextChildren] = splitChildren(
     height,
     contentArea,
     node,
+    pageEmpty,
   );
 
   return [
@@ -163,8 +200,15 @@ const splitView = (node: SafeNode, height: number, contentArea: number) => {
   ];
 };
 
-const split = (node: SafeNode, height: number, contentArea: number) =>
-  isText(node) ? splitText(node, height) : splitView(node, height, contentArea);
+const split = (
+  node: SafeNode,
+  height: number,
+  contentArea: number,
+  pageEmpty: boolean,
+) =>
+  isText(node)
+    ? splitText(node, height)
+    : splitView(node, height, contentArea, pageEmpty);
 
 const shouldResolveDynamicNodes = (node: SafeNode) => {
   const children = node.children || [];