[api-minor] Convert getAttachments to return data in a Map

Compared to regular `Object`s there's a number of advantages to using `Map`s:
 - 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 sort of data.
Furthermore, in PR 21351 the data returned by `getAttachments` changed slightly and third-party users will need to update their code anyway (hence why `[api-minor]` should be fine here).
This commit is contained in:
Jonas Jenwald 2026-06-09 10:17:23 +02:00
parent ea4fe68c01
commit ea139e7df1
4 changed files with 27 additions and 29 deletions

View File

@ -1136,21 +1136,21 @@ class Catalog {
/** /**
* Get attachments. * Get attachments.
* *
* @returns {Record<string, CatalogAttachment> | null} * @returns {Map<string, CatalogAttachment> | null}
* Attachments. * Attachments.
*/ */
get attachments() { get attachments() {
const obj = this.#catDict.get("Names"); const obj = this.#catDict.get("Names");
/** @type {Record<string, CatalogAttachment> | null} */ /** @type {Map<string, CatalogAttachment> | null} */
let attachments = null; let attachments = null;
if (obj instanceof Dict && obj.has("EmbeddedFiles")) { if (obj instanceof Dict && obj.has("EmbeddedFiles")) {
const nameTree = new NameTree(obj.getRaw("EmbeddedFiles"), this.xref); const nameTree = new NameTree(obj.getRaw("EmbeddedFiles"), this.xref);
for (const [key, value] of nameTree.getAll()) { for (const [key, value] of nameTree.getAll()) {
const fs = new FileSpec(value); (attachments ??= new Map()).set(
attachments ??= Object.create(null); stringToPDFString(key, /* keepEscapeSequence = */ true),
attachments[stringToPDFString(key, /* keepEscapeSequence = */ true)] = new FileSpec(value).serializable
fs.serializable; );
} }
} }
return shadow(this, "attachments", attachments); return shadow(this, "attachments", attachments);
@ -1880,7 +1880,7 @@ class Catalog {
if (docAttachments && id) { if (docAttachments && id) {
resultObj.attachmentId = id; resultObj.attachmentId = id;
resultObj.attachment = docAttachments[id]; resultObj.attachment = docAttachments.get(id);
// NOTE: the destination is relative to the *attachment*. // NOTE: the destination is relative to the *attachment*.
const attachmentDest = fetchRemoteDest(action); const attachmentDest = fetchRemoteDest(action);

View File

@ -838,7 +838,7 @@ class PDFDocumentProxy {
} }
/** /**
* @returns {Promise<Record<string, CatalogAttachment> | null>} * @returns {Promise<Map<string, CatalogAttachment> | null>}
* Promise that is resolved with a lookup table for mapping named * Promise that is resolved with a lookup table for mapping named
* attachments to their content. * attachments to their content.
*/ */
@ -3077,7 +3077,7 @@ class WorkerTransport {
} }
/** /**
* @returns {Promise<Record<string, CatalogAttachment> | null>} * @returns {Promise<Map<string, CatalogAttachment> | null>}
* Promise that is resolved with a lookup table for mapping named * Promise that is resolved with a lookup table for mapping named
* attachments to their content. * attachments to their content.
*/ */

View File

@ -1778,7 +1778,7 @@ describe("api", function () {
const pdfDoc = await loadingTask.promise; const pdfDoc = await loadingTask.promise;
const attachments = await pdfDoc.getAttachments(); const attachments = await pdfDoc.getAttachments();
expect(attachments["foo.txt"]).toEqual({ expect(attachments.get("foo.txt")).toEqual({
rawFilename: "foo.txt", rawFilename: "foo.txt",
filename: "foo.txt", filename: "foo.txt",
description: "", description: "",
@ -1797,7 +1797,8 @@ describe("api", function () {
const pdfDoc = await loadingTask.promise; const pdfDoc = await loadingTask.promise;
const attachments = await pdfDoc.getAttachments(); const attachments = await pdfDoc.getAttachments();
const { rawFilename, filename, description } = attachments["empty.pdf"]; const { rawFilename, filename, description } =
attachments.get("empty.pdf");
expect(rawFilename).toEqual("Empty page.pdf"); expect(rawFilename).toEqual("Empty page.pdf");
expect(filename).toEqual("Empty page.pdf"); expect(filename).toEqual("Empty page.pdf");
expect(description).toEqual( expect(description).toEqual(
@ -1827,11 +1828,11 @@ describe("api", function () {
const attachments = await pdfDoc.getAttachments(); const attachments = await pdfDoc.getAttachments();
const { description, filename, rawFilename } = const { description, filename, rawFilename } =
attachments["attachment.pdf"] || {}; attachments.get("attachment.pdf");
expect(rawFilename).toEqual("attachment.pdf"); expect(rawFilename).toEqual("attachment.pdf");
expect(filename).toEqual("attachment.pdf"); expect(filename).toEqual("attachment.pdf");
expect(description).toEqual(""); expect(description).toEqual("");
expect(attachments["attachment.pdf"].content).toBeUndefined(); expect(attachments.get("attachment.pdf").content).toBeUndefined();
const content = await pdfDoc.getAttachmentContent("attachment.pdf"); const content = await pdfDoc.getAttachmentContent("attachment.pdf");
expect(passwordRequests).toEqual(1); expect(passwordRequests).toEqual(1);
@ -1861,11 +1862,11 @@ describe("api", function () {
const attachments = await pdfDoc.getAttachments(); const attachments = await pdfDoc.getAttachments();
const { description, filename, rawFilename } = const { description, filename, rawFilename } =
attachments["attachment.pdf"] || {}; attachments.get("attachment.pdf");
expect(rawFilename).toEqual("attachment.pdf"); expect(rawFilename).toEqual("attachment.pdf");
expect(filename).toEqual("attachment.pdf"); expect(filename).toEqual("attachment.pdf");
expect(description).toEqual(""); expect(description).toEqual("");
expect(attachments["attachment.pdf"].content).toBeUndefined(); expect(attachments.get("attachment.pdf").content).toBeUndefined();
const content = await pdfDoc.getAttachmentContent("attachment.pdf"); const content = await pdfDoc.getAttachmentContent("attachment.pdf");
expect(reasons).toEqual([ expect(reasons).toEqual([
@ -1888,7 +1889,7 @@ describe("api", function () {
try { try {
const pdfDoc = await loadingTask.promise; const pdfDoc = await loadingTask.promise;
const attachments = await pdfDoc.getAttachments(); const attachments = await pdfDoc.getAttachments();
const attachment = attachments?.["attachment.pdf"]; const attachment = attachments.get("attachment.pdf");
expect(attachment).toBeDefined(); expect(attachment).toBeDefined();
expect(attachment.filename).toEqual("attachment.pdf"); expect(attachment.filename).toEqual("attachment.pdf");
@ -6996,7 +6997,7 @@ small scripts as well as for`);
// Verify the original document has the expected attachment. // Verify the original document has the expected attachment.
const originalAttachments = await pdfDoc.getAttachments(); const originalAttachments = await pdfDoc.getAttachments();
expect(originalAttachments["foo.txt"]).toBeDefined(); expect(originalAttachments.get("foo.txt")).toBeDefined();
const data = await pdfDoc.extractPages([ const data = await pdfDoc.extractPages([
{ document: null, includePages: [0] }, { document: null, includePages: [0] },
@ -7008,7 +7009,7 @@ small scripts as well as for`);
const attachments = await pdfDoc.getAttachments(); const attachments = await pdfDoc.getAttachments();
expect(attachments).not.toBeNull(); expect(attachments).not.toBeNull();
expect(attachments["foo.txt"]).toEqual({ expect(attachments.get("foo.txt")).toEqual({
rawFilename: "foo.txt", rawFilename: "foo.txt",
filename: "foo.txt", filename: "foo.txt",
description: "", description: "",
@ -7043,12 +7044,12 @@ small scripts as well as for`);
const expectedContent = new Uint8Array([ const expectedContent = new Uint8Array([
98, 97, 114, 32, 98, 97, 122, 32, 10, 98, 97, 114, 32, 98, 97, 122, 32, 10,
]); ]);
expect(attachments["foo.txt"]).toEqual({ expect(attachments.get("foo.txt")).toEqual({
rawFilename: "foo.txt", rawFilename: "foo.txt",
filename: "foo.txt", filename: "foo.txt",
description: "", description: "",
}); });
expect(attachments["foo.txt_1"]).toEqual({ expect(attachments.get("foo.txt_1")).toEqual({
rawFilename: "foo.txt", rawFilename: "foo.txt",
filename: "foo.txt", filename: "foo.txt",
description: "", description: "",

View File

@ -39,7 +39,7 @@ import { waitOnEventOrTimeout } from "./event_utils.js";
/** /**
* @typedef PDFAttachmentViewerRenderParameters * @typedef PDFAttachmentViewerRenderParameters
* @property {Record<string, CatalogAttachment> | null} attachments - A lookup * @property {Map<string, CatalogAttachment> | null} attachments - A lookup
* table of attachment objects. * table of attachment objects.
* @property {boolean} [keepRenderedCapability] * @property {boolean} [keepRenderedCapability]
*/ */
@ -154,8 +154,7 @@ class PDFAttachmentViewer extends BaseTreeViewer {
const ul = document.createElement("ul"); const ul = document.createElement("ul");
fragment.append(ul); fragment.append(ul);
let attachmentsCount = 0; let attachmentsCount = 0;
for (const name in attachments) { for (const [name, item] of attachments) {
const item = attachments[name];
const li = document.createElement("li"); const li = document.createElement("li");
ul.append(li); ul.append(li);
const element = document.createElement("a"); const element = document.createElement("a");
@ -182,14 +181,12 @@ class PDFAttachmentViewer extends BaseTreeViewer {
if (renderedPromise !== this._renderedCapability.promise) { if (renderedPromise !== this._renderedCapability.promise) {
return; // The FileAttachment annotation belongs to a previous document. return; // The FileAttachment annotation belongs to a previous document.
} }
const attachments = this._attachments || Object.create(null); const attachments = new Map(this._attachments);
for (const name in attachments) { if (attachments.has(item.filename)) {
if (item.filename === name) {
return; // Ignore the new attachment if it already exists. return; // Ignore the new attachment if it already exists.
} }
} attachments.set(item.filename, item);
attachments[item.filename] = item;
this.render({ this.render({
attachments, attachments,