fix: fixed unbound lineHeight during rendering, resolves #3083 and #3402

Author: Silvenga

packages/layout/src/text/getAttributedString.ts

@@ -1,6 +1,7 @@
 import * as P from '@react-pdf/primitives';
 import { Fragment, fromFragments } from '@react-pdf/textkit';
 import FontStore from '@react-pdf/font';
+import { parseFloat } from '@react-pdf/fns';
 
 import { embedEmojis } from './emoji';
 import ignoreChars from './ignoreChars';
@@ -82,7 +83,7 @@ const getFragments = (
     color,
     opacity,
     fontSize,
-    lineHeight,
+    lineHeight: parseFloat(lineHeight),
     direction,
     verticalAlign,
     backgroundColor,

packages/layout/tests/steps/lineHeightRenderProp.test.ts

@@ -0,0 +1,114 @@
+import { describe, expect, test } from 'vitest';
+import * as P from '@react-pdf/primitives';
+import FontStore from '@react-pdf/font';
+import { parseFloat } from '@react-pdf/fns';
+
+import { loadYoga } from '../../src/yoga';
+import resolvePagination from '../../src/steps/resolvePagination';
+import resolveDimensions from '../../src/steps/resolveDimensions';
+import { SafeDocumentNode, SafeNode, SafeTextNode } from '../../src/types';
+
+const fontStore = new FontStore();
+
+const isText = (node: SafeNode): node is SafeTextNode => node.type === P.Text;
+
+const calcLayout = (node: SafeDocumentNode) =>
+  resolvePagination(resolveDimensions(node, fontStore), fontStore);
+
+describe('lineHeight + render prop bug (issues #3083, #3402, #2988)', () => {
+  test('should render dynamic text when lineHeight is set on page', async () => {
+    const yoga = await loadYoga();
+
+    const layout = calcLayout({
+      type: 'DOCUMENT',
+      yoga,
+      props: {},
+      children: [
+        {
+          type: 'PAGE',
+          props: {},
+          style: {
+            width: 100,
+            height: 200,
+            fontSize: 9,
+            lineHeight: 1.5,
+          },
+          children: [
+            {
+              type: 'TEXT',
+              style: {},
+              props: {},
+              children: [
+                {
+                  type: 'TEXT_INSTANCE',
+                  value: 'static text',
+                },
+              ],
+            },
+            {
+              type: 'TEXT',
+              style: {},
+              props: {
+                render: () => 'dynamic text',
+              },
+              children: [],
+            },
+          ],
+        },
+      ],
+    });
+
+    const page = layout.children[0];
+    const children = page.children!;
+    const textNodes = children.filter(isText);
+    const staticText = textNodes[0];
+    const dynamicText = textNodes[1];
+
+    expect(staticText.lines).toBeDefined();
+    expect(dynamicText.lines).toBeDefined();
+    expect(dynamicText.lines!.length).toBeGreaterThan(0);
+    const firstLine = dynamicText.lines![0];
+    expect(firstLine.string).toContain('dynamic text');
+
+    // lineHeight 1.5 * fontSize 9 = 13.5; double-multiplied would be 121.5
+    expect(dynamicText.box!.height).toBeLessThan(50);
+  });
+
+  test('should not double-multiply lineHeight on dynamic text nodes', async () => {
+    const yoga = await loadYoga();
+
+    const layout = calcLayout({
+      type: 'DOCUMENT',
+      yoga,
+      props: {},
+      children: [
+        {
+          type: 'PAGE',
+          props: {},
+          style: {
+            width: 100,
+            height: 200,
+            fontSize: 10,
+            lineHeight: 1.5,
+          },
+          children: [
+            {
+              type: 'TEXT',
+              style: {},
+              props: {
+                render: () => 'hello',
+              },
+              children: [],
+            },
+          ],
+        },
+      ],
+    });
+
+    const page = layout.children[0];
+    const dynamicText = page.children!.find(isText)!;
+
+    const lineHeight = parseFloat(String(dynamicText.style!.lineHeight));
+    expect(lineHeight).toBe(15);
+  });
+});

packages/renderer/tests/lineHeightRenderProp.test.jsx

@@ -0,0 +1,79 @@
+import { describe, expect, test } from 'vitest';
+import { getDocument } from 'pdfjs-dist/legacy/build/pdf.mjs';
+
+import {
+  Document,
+  Page,
+  Text,
+  View,
+  StyleSheet,
+  renderToBuffer,
+} from '@react-pdf/renderer';
+
+const styles = StyleSheet.create({
+  page: {
+    lineHeight: 1.5,
+    fontSize: 9,
+  },
+});
+
+const getTextContent = async (pdfBuffer) => {
+  const document = await getDocument({
+    data: new Uint8Array(pdfBuffer),
+    verbosity: 0,
+  }).promise;
+
+  const page = await document.getPage(1);
+  const content = await page.getTextContent();
+
+  return content.items.map((item) => item.str).join(' ');
+};
+
+describe('lineHeight + render prop end-to-end (issues #3083, #3402, #2988)', () => {
+  test('should render dynamic text when lineHeight is set on page', async () => {
+    const doc = (
+      <Document>
+        <Page size="LETTER" style={styles.page}>
+          <Text>Static text renders fine</Text>
+          <View fixed>
+            <Text
+              render={({ pageNumber, totalPages }) =>
+                `${pageNumber} / ${totalPages}`
+              }
+            />
+          </View>
+        </Page>
+      </Document>
+    );
+
+    const buffer = await renderToBuffer(doc);
+    const text = await getTextContent(buffer);
+
+    // Both static and dynamic text should be present
+    expect(text).toContain('Static text renders fine');
+    expect(text).toContain('1 / 1');
+  });
+
+  test('should render dynamic text without lineHeight (control test)', async () => {
+    const doc = (
+      <Document>
+        <Page size="LETTER" style={{ fontSize: 9 }}>
+          <Text>Static text renders fine</Text>
+          <View fixed>
+            <Text
+              render={({ pageNumber, totalPages }) =>
+                `${pageNumber} / ${totalPages}`
+              }
+            />
+          </View>
+        </Page>
+      </Document>
+    );
+
+    const buffer = await renderToBuffer(doc);
+    const text = await getTextContent(buffer);
+
+    expect(text).toContain('Static text renders fine');
+    expect(text).toContain('1 / 1');
+  });
+});

packages/stylesheet/src/resolve/text.ts

@@ -60,10 +60,17 @@ const transformLineHeight = (
 
   // Percent values: use this number multiplied by the element's font size
   const { percent } = matchPercent(lineHeight) || {};
-  if (percent) return percent * fontSize;
+  if (percent) return `${percent * fontSize}pt`;
 
-  // Unitless values: use this number multiplied by the element's font size
-  return isNaN(Number(value)) ? lineHeight : lineHeight * fontSize;
+  // Values with units (e.g. '20px') are already absolute. Non-numeric keyword
+  // strings (e.g. 'normal') pass through transformUnit unchanged and should
+  // not have 'pt' appended.
+  // Note that renderer doesn't support keywords, so this is purely defensive.
+  if (isNaN(Number(value))) {
+    return typeof lineHeight === 'number' ? `${lineHeight}pt` : lineHeight;
+  }
+
+  return `${lineHeight * fontSize}pt`;
 };
 
 const processLineHeight = <K extends StyleKey>(

packages/stylesheet/src/types.ts

@@ -346,7 +346,7 @@ export type TextSafeStyle = TextExpandedStyle & {
   fontSize?: number;
   fontWeight?: number;
   letterSpacing?: number;
-  lineHeight?: number;
+  lineHeight?: number | string;
 };
 
 // Margins

packages/stylesheet/tests/text.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, test } from 'vitest';
+import { parseFloat } from '@react-pdf/fns';
 
 import resolve from '../src/resolve';
 
@@ -130,49 +131,49 @@ describe('resolve stylesheet text', () => {
   test('should resolve line height number', () => {
     const styles = resolveStyle({ lineHeight: 2 });
 
-    expect(styles.lineHeight).toBe(18 * 2);
+    expect(styles.lineHeight).toBe(`${18 * 2}pt`);
   });
 
   test('should resolve number line height with font size', () => {
     const styles = resolveStyle({ lineHeight: 2, fontSize: 10 });
 
-    expect(styles.lineHeight).toBe(10 * 2);
+    expect(styles.lineHeight).toBe(`${10 * 2}pt`);
   });
 
   test('should resolve string line height', () => {
     const styles = resolveStyle({ lineHeight: '2' });
 
-    expect(styles.lineHeight).toBe(18 * 2);
+    expect(styles.lineHeight).toBe(`${18 * 2}pt`);
   });
 
   test('should resolve string line height  with font-size', () => {
     const styles = resolveStyle({ lineHeight: '2', fontSize: 10 });
 
-    expect(styles.lineHeight).toBe(10 * 2);
+    expect(styles.lineHeight).toBe(`${10 * 2}pt`);
   });
 
   test('should resolve percentage line height', () => {
     const styles = resolveStyle({ lineHeight: '200%' });
 
-    expect(styles.lineHeight).toBe(18 * 2);
+    expect(styles.lineHeight).toBe(`${18 * 2}pt`);
   });
 
   test('should resolve percentage line height with font-size', () => {
     const styles = resolveStyle({ lineHeight: '200%', fontSize: 10 });
 
-    expect(styles.lineHeight).toBe(10 * 2);
+    expect(styles.lineHeight).toBe(`${10 * 2}pt`);
   });
 
   test('should resolve px line height', () => {
     const styles = resolveStyle({ lineHeight: '20px' });
 
-    expect(styles.lineHeight).toBe(20);
+    expect(styles.lineHeight).toBe(`${20}pt`);
   });
 
   test('should resolve mm line height', () => {
     const styles = resolveStyle({ lineHeight: '20mm' });
 
-    expect(styles.lineHeight).toBeCloseTo(56.69, 1);
+    expect(parseFloat(styles.lineHeight)).toBeCloseTo(56.69, 1);
   });
 
   test('should resolve font family', () => {
@@ -328,13 +329,13 @@ describe('resolve stylesheet text', () => {
   test('should resolve line height rem units', () => {
     const styles = resolveStyle({ lineHeight: '2rem' });
 
-    expect(styles).toEqual({ lineHeight: 20 });
+    expect(styles).toEqual({ lineHeight: '20pt' });
   });
 
   test('should resolve line height in units', () => {
     const styles = resolveStyle({ lineHeight: '0.5in' });
 
-    expect(styles).toEqual({ lineHeight: 36 });
+    expect(styles).toEqual({ lineHeight: '36pt' });
   });
 
   test('should resolve string max lines', () => {