From 6fbd2227db9ad34ed22fcd2a34f158ffedad8360 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 19 Jul 2026 12:07:08 +0200 Subject: [PATCH] [api-minor] Convert `getDestinations` to return data in a Map Compared to regular Objects there's a number of advantages to using Maps: - They support proper iteration. - They have a simple way to check for the existence of data. - They have a simple/efficient way to check the number of elements. If this functionality was added today, I cannot imagine that we'd choose an Object for this data. --- src/core/catalog.js | 31 +++++------ src/core/editor/pdf_editor.js | 2 +- src/display/api.js | 6 +- test/unit/api_spec.js | 101 ++++++++++++++++++++-------------- 4 files changed, 77 insertions(+), 63 deletions(-) diff --git a/src/core/catalog.js b/src/core/catalog.js index e637b744c..59d571b02 100644 --- a/src/core/catalog.js +++ b/src/core/catalog.js @@ -763,15 +763,17 @@ class Catalog { } get destinations() { - const rawDests = this.#readDests(), - dests = Object.create(null); - for (const obj of rawDests) { + const dests = new Map(); + + for (const obj of this.#readDests()) { if (obj instanceof NameTree) { for (const [key, value] of obj.getAll()) { const dest = fetchDest(value); if (dest) { - dests[stringToPDFString(key, /* keepEscapeSequence = */ true)] = - dest; + dests.set( + stringToPDFString(key, /* keepEscapeSequence = */ true), + dest + ); } } } else if (obj instanceof Dict) { @@ -779,8 +781,10 @@ class Catalog { const dest = fetchDest(value); if (dest) { // Always let the NameTree take precedence. - dests[stringToPDFString(key, /* keepEscapeSequence = */ true)] ||= - dest; + dests.getOrInsert( + stringToPDFString(key, /* keepEscapeSequence = */ true), + dest + ); } } } @@ -791,11 +795,10 @@ class Catalog { getDestination(id) { // Avoid extra lookup/parsing when all destinations are already available. if (Object.hasOwn(this, "destinations")) { - return this.destinations[id] ?? null; + return this.destinations.get(id) ?? null; } - const rawDests = this.#readDests(); - for (const obj of rawDests) { + for (const obj of this.#readDests()) { if (obj instanceof NameTree || obj instanceof Dict) { const dest = fetchDest(obj.get(id)); if (dest) { @@ -807,13 +810,7 @@ class Catalog { // Always fallback to checking all destinations, in order to support: // - PDF documents with out-of-order NameTrees (fixes issue 10272). // - Destination keys that use PDFDocEncoding (fixes issue 19835). - if (rawDests.length) { - const dest = this.destinations[id]; - if (dest) { - return dest; - } - } - return null; + return this.destinations.get(id) ?? null; } #readDests() { diff --git a/src/core/editor/pdf_editor.js b/src/core/editor/pdf_editor.js index 1cfff4200..fcc8edf36 100644 --- a/src/core/editor/pdf_editor.js +++ b/src/core/editor/pdf_editor.js @@ -1615,7 +1615,7 @@ class PDFEditor { } const { destinations, pagesMap } = documentData; const newDestinations = (documentData.destinations = new Map()); - for (const [key, dest] of Object.entries(destinations)) { + for (const [key, dest] of destinations) { const pageRef = dest[0]; const pageData = pageRef instanceof Ref && pagesMap.get(pageRef); if (!pageData) { diff --git a/src/display/api.js b/src/display/api.js index 550b87a16..77d055bc0 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -775,7 +775,7 @@ class PDFDocumentProxy { } /** - * @returns {Promise>>} A promise that is resolved + * @returns {Promise>>} A promise that is resolved * with a mapping from named destinations to references. * * This can be slow for large documents. Use `getDestination` instead. @@ -3085,9 +3085,7 @@ class WorkerTransport { if (typeof id !== "string") { return Promise.reject(new Error("Invalid destination request.")); } - return this.messageHandler.sendWithPromise("GetDestination", { - id, - }); + return this.messageHandler.sendWithPromise("GetDestination", { id }); } getPageLabels() { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index ce44fd7f3..513364a43 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -1467,9 +1467,11 @@ describe("api", function () { it("gets destinations, from /Dests dictionary", async function () { const destinations = await pdfDocument.getDestinations(); - expect(destinations).toEqual({ - chapter1: [{ gen: 0, num: 17 }, { name: "XYZ" }, 0, 841.89, null], - }); + expect(destinations).toEqual( + new Map([ + ["chapter1", [{ gen: 0, num: 17 }, { name: "XYZ" }, 0, 841.89, null]], + ]) + ); }); it("gets a destination, from /Dests dictionary", async function () { @@ -1494,10 +1496,12 @@ describe("api", function () { const loadingTask = getDocument(buildGetDocumentParams("issue6204.pdf")); const pdfDoc = await loadingTask.promise; const destinations = await pdfDoc.getDestinations(); - expect(destinations).toEqual({ - "Page.1": [{ num: 1, gen: 0 }, { name: "XYZ" }, 0, 375, null], - "Page.2": [{ num: 6, gen: 0 }, { name: "XYZ" }, 0, 375, null], - }); + expect(destinations).toEqual( + new Map([ + ["Page.1", [{ num: 1, gen: 0 }, { name: "XYZ" }, 0, 375, null]], + ["Page.2", [{ num: 6, gen: 0 }, { name: "XYZ" }, 0, 375, null]], + ]) + ); await loadingTask.destroy(); }); @@ -1506,11 +1510,13 @@ describe("api", function () { const loadingTask = getDocument(buildGetDocumentParams("issue19474.pdf")); const pdfDoc = await loadingTask.promise; const destinations = await pdfDoc.getDestinations(); - expect(destinations).toEqual({ - A: [{ num: 1, gen: 0 }, { name: "Fit" }], - B: [{ num: 4, gen: 0 }, { name: "Fit" }], - C: [{ num: 5, gen: 0 }, { name: "Fit" }], - }); + expect(destinations).toEqual( + new Map([ + ["A", [{ num: 1, gen: 0 }, { name: "Fit" }]], + ["B", [{ num: 4, gen: 0 }, { name: "Fit" }]], + ["C", [{ num: 5, gen: 0 }, { name: "Fit" }]], + ]) + ); await loadingTask.destroy(); }); @@ -6418,7 +6424,10 @@ small scripts as well as for`); expect(annotations.length).toEqual(2); expect(annotations[0].dest).toEqual("foo"); expect(annotations[1].unsafeUrl).toEqual("other.pdf#target"); - expect(Object.keys(await pdfDoc.getDestinations())).toEqual(["foo"]); + + const destinations = await pdfDoc.getDestinations(); + expect([...destinations.keys()]).toEqual(["foo"]); + await loadingTask.destroy(); }); @@ -6435,7 +6444,8 @@ small scripts as well as for`); ]; let loadingTask = getDocument({ data: assemblePdf(objects) }); let pdfDoc = await loadingTask.promise; - expect(Object.keys(await pdfDoc.getDestinations())) + let destinations = await pdfDoc.getDestinations(); + expect([...destinations.keys()]) .withContext("before extraction") .toEqual(["名"]); const data = await pdfDoc.extractPages([{ document: null }]); @@ -6443,7 +6453,8 @@ small scripts as well as for`); loadingTask = getDocument({ data }); pdfDoc = await loadingTask.promise; - expect(Object.keys(await pdfDoc.getDestinations())) + destinations = await pdfDoc.getDestinations(); + expect([...destinations.keys()]) .withContext("after extraction") .toEqual(["名"]); await loadingTask.destroy(); @@ -6456,7 +6467,7 @@ small scripts as well as for`); let pdfDoc = await loadingTask.promise; let destinations = await pdfDoc.getDestinations(); - expect(Object.keys(destinations).sort()).toEqual(["foo", "foo_p2"]); + expect([...destinations.keys()].sort()).toEqual(["foo", "foo_p2"]); const data = await pdfDoc.extractPages([ { document: null }, @@ -6468,7 +6479,7 @@ small scripts as well as for`); pdfDoc = await loadingTask.promise; destinations = await pdfDoc.getDestinations(); - expect(Object.keys(destinations).sort()).toEqual([ + expect([...destinations.keys()].sort()).toEqual([ "foo", "foo_p2", "foo_p2_1", @@ -6489,10 +6500,12 @@ small scripts as well as for`); let pdfDoc = await loadingTask.promise; let pagesRef = await getPageRefs(pdfDoc); let destinations = await pdfDoc.getDestinations(); - expect(destinations).toEqual({ - "Page.1": [pagesRef[0], { name: "XYZ" }, 0, 375, null], - "Page.2": [pagesRef[1], { name: "XYZ" }, 0, 375, null], - }); + expect(destinations).toEqual( + new Map([ + ["Page.1", [pagesRef[0], { name: "XYZ" }, 0, 375, null]], + ["Page.2", [pagesRef[1], { name: "XYZ" }, 0, 375, null]], + ]) + ); let data = await pdfDoc.extractPages([ { document: null }, @@ -6507,12 +6520,14 @@ small scripts as well as for`); pagesRef = await getPageRefs(pdfDoc); destinations = await pdfDoc.getDestinations(); - expect(destinations).toEqual({ - "Page.1": [pagesRef[0], { name: "XYZ" }, 0, 375, null], - "Page.2": [pagesRef[1], { name: "XYZ" }, 0, 375, null], - "Page.1_p3": [pagesRef[2], { name: "XYZ" }, 0, 375, null], - "Page.2_p4": [pagesRef[3], { name: "XYZ" }, 0, 375, null], - }); + expect(destinations).toEqual( + new Map([ + ["Page.1", [pagesRef[0], { name: "XYZ" }, 0, 375, null]], + ["Page.2", [pagesRef[1], { name: "XYZ" }, 0, 375, null]], + ["Page.1_p3", [pagesRef[2], { name: "XYZ" }, 0, 375, null]], + ["Page.2_p4", [pagesRef[3], { name: "XYZ" }, 0, 375, null]], + ]) + ); const expectedDests = ["Page.2", "Page.1", "Page.2_p4", "Page.1_p3"]; for (let i = 1; i <= 4; i++) { const pdfPage = await pdfDoc.getPage(i); @@ -6534,16 +6549,18 @@ small scripts as well as for`); pagesRef = await getPageRefs(pdfDoc); destinations = await pdfDoc.getDestinations(); - expect(destinations).toEqual({ - "Page.1": [pagesRef[0], { name: "XYZ" }, 0, 375, null], - "Page.2": [pagesRef[1], { name: "XYZ" }, 0, 375, null], - "Page.1_p3": [pagesRef[2], { name: "XYZ" }, 0, 375, null], - "Page.2_p4": [pagesRef[3], { name: "XYZ" }, 0, 375, null], - "Page.1_p5": [pagesRef[4], { name: "XYZ" }, 0, 375, null], - "Page.2_p6": [pagesRef[5], { name: "XYZ" }, 0, 375, null], - "Page.1_p3_p7": [pagesRef[6], { name: "XYZ" }, 0, 375, null], - "Page.2_p4_p8": [pagesRef[7], { name: "XYZ" }, 0, 375, null], - }); + expect(destinations).toEqual( + new Map([ + ["Page.1", [pagesRef[0], { name: "XYZ" }, 0, 375, null]], + ["Page.2", [pagesRef[1], { name: "XYZ" }, 0, 375, null]], + ["Page.1_p3", [pagesRef[2], { name: "XYZ" }, 0, 375, null]], + ["Page.2_p4", [pagesRef[3], { name: "XYZ" }, 0, 375, null]], + ["Page.1_p5", [pagesRef[4], { name: "XYZ" }, 0, 375, null]], + ["Page.2_p6", [pagesRef[5], { name: "XYZ" }, 0, 375, null]], + ["Page.1_p3_p7", [pagesRef[6], { name: "XYZ" }, 0, 375, null]], + ["Page.2_p4_p8", [pagesRef[7], { name: "XYZ" }, 0, 375, null]], + ]) + ); expectedDests.push( "Page.2_p6", "Page.1_p5", @@ -6575,10 +6592,12 @@ small scripts as well as for`); const pagesRef = await getPageRefs(pdfDoc); const destinations = await pdfDoc.getDestinations(); - expect(destinations).toEqual({ - "Page.1": [pagesRef[0], { name: "XYZ" }, 0, 375, null], - "Page.2": [pagesRef[1], { name: "XYZ" }, 0, 375, null], - }); + expect(destinations).toEqual( + new Map([ + ["Page.1", [pagesRef[0], { name: "XYZ" }, 0, 375, null]], + ["Page.2", [pagesRef[1], { name: "XYZ" }, 0, 375, null]], + ]) + ); const pdfPage = await pdfDoc.getPage(3); const annots = await pdfPage.getAnnotations(); expect(annots.length).toEqual(0);