Fix page count bug (#153)

* Initialize PDFDocument.pageCount correctly

* Lazily initialize PDFDocument.pageCount

* Add unit tests for PDFDocument.getPageCount()

* Fix page count caching bug

scratchpad/index.ts

@@ -1,47 +1,19 @@
 import fs from 'fs';
-import fetch from 'node-fetch';
 import { openPdf, Reader } from './open';
 
-import fontkit from '@pdf-lib/fontkit';
-import { PDFDocument, rgb } from 'src/index';
+import { PDFDocument } from 'src/index';
 
-async function embedImages() {
-  const url = 'https://pdf-lib.js.org/assets/ubuntu/Ubuntu-R.ttf';
-  const fontBytes = await fetch(url).then((res) => res.arrayBuffer());
+(async () => {
+  const pdfDoc = await PDFDocument.load(
+    fs.readFileSync('assets/pdfs/normal.pdf'),
+  );
 
-  const pdfDoc = await PDFDocument.create();
-
-  pdfDoc.registerFontkit(fontkit);
-  const customFont = await pdfDoc.embedFont(fontBytes);
-
-  const page = pdfDoc.addPage();
-
-  const text = 'This is text in an embedded font!';
-  const textSize = 35;
-  const textWidth = customFont.widthOfTextAtSize(text, textSize);
-  const textHeight = customFont.heightAtSize(textSize);
-
-  page.drawText(text, {
-    x: 40,
-    y: 450,
-    size: textSize,
-    font: customFont,
-    color: rgb(0, 0.53, 0.71),
-  });
-  page.drawRectangle({
-    x: 40,
-    y: 450,
-    width: textWidth,
-    height: textHeight,
-    borderColor: rgb(1, 0, 0),
-    borderWidth: 1.5,
-  });
+  console.log('Count:', pdfDoc.getPageCount());
+  pdfDoc.removePage(1);
 
   const pdfBytes = await pdfDoc.save();
 
   fs.writeFileSync('./out.pdf', pdfBytes);
 
   openPdf('./out.pdf', Reader.Preview);
-}
-
-embedImages();
+})();

src/api/PDFDocument.ts

@@ -159,7 +159,7 @@ export default class PDFDocument {
   readonly isEncrypted: boolean;
 
   private fontkit?: Fontkit;
-  private pageCount: number;
+  private pageCount: number | undefined;
   private readonly pageCache: Cache<PDFPage[]>;
   private readonly pageMap: Map<PDFPageLeaf, PDFPage>;
   private readonly fonts: PDFFont[];
@@ -178,8 +178,6 @@ export default class PDFDocument {
     this.fonts = [];
     this.images = [];
 
-    this.pageCount = this.getPageCount();
-
     if (!ignoreEncryption && this.isEncrypted) throw new EncryptedPDFError();
   }
 
@@ -204,6 +202,7 @@ export default class PDFDocument {
    * @returns The number of pages in this document.
    */
   getPageCount(): number {
+    if (this.pageCount === undefined) this.pageCount = this.getPages().length;
     return this.pageCount;
   }
 
@@ -254,11 +253,11 @@ export default class PDFDocument {
    * @param index The index of the page to be removed.
    */
   removePage(index: number): void {
-    const pageCount = this.getPages().length;
-    if (pageCount === 0) throw new RemovePageFromEmptyDocumentError();
+    const pageCount = this.getPageCount();
+    if (this.pageCount === 0) throw new RemovePageFromEmptyDocumentError();
     assertRange(index, 'index', 0, pageCount - 1);
     this.catalog.removeLeafNode(index);
-    this.pageCount -= 1;
+    this.pageCount = pageCount - 1;
   }
 
   /**
@@ -294,9 +293,7 @@ export default class PDFDocument {
    */
   addPage(page?: PDFPage | [number, number]): PDFPage {
     assertIs(page, 'page', ['undefined', [PDFPage, 'PDFPage'], Array]);
-    const pages = this.getPages();
-    this.pageCount += 1;
-    return this.insertPage(pages.length, page);
+    return this.insertPage(this.getPageCount() - 1, page);
   }
 
   /**
@@ -332,7 +329,8 @@ export default class PDFDocument {
    * @returns The newly created (or existing) page.
    */
   insertPage(index: number, page?: PDFPage | [number, number]): PDFPage {
-    assertRange(index, 'index', 0, this.pageCount - 1);
+    const pageCount = this.getPageCount();
+    assertRange(index, 'index', 0, pageCount - 1);
     assertIs(page, 'page', ['undefined', [PDFPage, 'PDFPage'], Array]);
     if (!page || Array.isArray(page)) {
       const dims = Array.isArray(page) ? page : PageSizes.A4;
@@ -348,7 +346,7 @@ export default class PDFDocument {
     this.pageMap.set(page.node, page);
     this.pageCache.invalidate();
 
-    this.pageCount += 1;
+    this.pageCount = pageCount + 1;
 
     return page;
   }
@@ -608,7 +606,7 @@ export default class PDFDocument {
     assertIs(addDefaultPage, 'addDefaultPage', ['boolean']);
     assertIs(objectsPerTick, 'objectsPerTick', ['number']);
 
-    if (addDefaultPage && this.getPages().length === 0) this.addPage();
+    if (addDefaultPage && this.pageCount === 0) this.addPage();
     await this.flush();
 
     const Writer = useObjectStreams ? PDFStreamWriter : PDFWriter;

src/utils/validators.ts

@@ -6,6 +6,7 @@ export const getType = (val: any) => {
   if (val === null) return 'null';
   if (val === undefined) return 'undefined';
   if (typeof val === 'string') return 'string';
+  if (isNaN(val)) return 'NaN';
   if (typeof val === 'number') return 'number';
   if (typeof val === 'boolean') return 'boolean';
   if (typeof val === 'symbol') return 'symbol';
@@ -33,7 +34,7 @@ export const isType = (value: any, type: TypeDescriptor) => {
   if (type === 'null') return value === null;
   if (type === 'undefined') return value === undefined;
   if (type === 'string') return typeof value === 'string';
-  if (type === 'number') return typeof value === 'number';
+  if (type === 'number') return typeof value === 'number' && !isNaN(value);
   if (type === 'boolean') return typeof value === 'boolean';
   if (type === 'symbol') return typeof value === 'symbol';
   if (type === 'bigint') return typeof value === 'bigint';
@@ -107,6 +108,8 @@ export const assertRange = (
   max: number,
 ) => {
   assertIs(value, valueName, ['number']);
+  assertIs(min, 'min', ['number']);
+  assertIs(max, 'max', ['number']);
   if (value < min || value > max) {
     // prettier-ignore
     throw new Error(`${backtick(valueName)} must be at least ${min} and at most ${max}, but was actually ${value}`);

tests/api/PDFDocument.spec.ts

@@ -88,4 +88,40 @@ describe(`PDFDocument`, () => {
       expect(pdfDoc.isEncrypted).toBe(true);
     });
   });
+
+  describe(`getPageCount() method`, () => {
+    let pdfDoc: PDFDocument;
+    beforeAll(async () => {
+      const bytes = fs.readFileSync('assets/pdfs/normal.pdf');
+      const parseSpeed = ParseSpeeds.Fastest;
+      pdfDoc = await PDFDocument.load(bytes, { parseSpeed });
+    });
+
+    it(`returns the initial page count of the document`, () => {
+      expect(pdfDoc.getPageCount()).toBe(2);
+    });
+
+    it(`returns the updated page count after adding pages`, () => {
+      pdfDoc.addPage();
+      pdfDoc.addPage();
+      expect(pdfDoc.getPageCount()).toBe(4);
+    });
+
+    it(`returns the updated page count after inserting pages`, () => {
+      pdfDoc.insertPage(0);
+      pdfDoc.insertPage(4);
+      expect(pdfDoc.getPageCount()).toBe(6);
+    });
+
+    it(`returns the updated page count after removing pages`, () => {
+      pdfDoc.removePage(5);
+      pdfDoc.removePage(0);
+      expect(pdfDoc.getPageCount()).toBe(4);
+    });
+
+    it(`returns 0 for brand new documents`, async () => {
+      const newDoc = await PDFDocument.create();
+      expect(newDoc.getPageCount()).toBe(0);
+    });
+  });
 });