fix: allocate bookmark numeric ids per document

src/file/file.ts

@@ -21,6 +21,7 @@ import type { Footer, Header } from "./header";
 import { HeaderWrapper, type IDocumentHeader } from "./header-wrapper";
 import { Media } from "./media";
 import { Numbering } from "./numbering";
+import { BookmarkIds } from "./paragraph/links/bookmark-ids";
 import { Comments } from "./paragraph/run/comment-run";
 import { CommentsExtended, CommentsIds } from "./paragraph/run/comments-extended";
 import { Relationships } from "./relationships";
@@ -156,6 +157,7 @@ export class File {
     private readonly coreProperties: CoreProperties;
     private readonly numbering: Numbering;
     private readonly media: Media;
+    private readonly bookmarkIds = new BookmarkIds();
     private readonly fileRelationships: Relationships;
     private readonly footnotesWrapper: FootnotesWrapper;
     private readonly endnotesWrapper: EndnotesWrapper;
@@ -430,6 +432,10 @@ export class File {
         return this.media;
     }
 
+    public get BookmarkIds(): BookmarkIds {
+        return this.bookmarkIds;
+    }
+
     public get FileRelationships(): Relationships {
         return this.fileRelationships;
     }

src/file/paragraph/links/bookmark-ids.spec.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from "vitest";
+
+import { BookmarkIds } from "./bookmark-ids";
+
+describe("BookmarkIds", () => {
+    it("should number bookmarks from one, in the order they are asked for", () => {
+        const ids = new BookmarkIds();
+
+        expect(ids.getId("first")).to.equal(1);
+        expect(ids.getId("second")).to.equal(2);
+        expect(ids.getId("third")).to.equal(3);
+    });
+
+    it("should return the same id for the same name, so start and end markers pair", () => {
+        const ids = new BookmarkIds();
+
+        const first = ids.getId("intro");
+        ids.getId("other");
+
+        expect(ids.getId("intro")).to.equal(first);
+    });
+
+    it("should start again at one for a new instance, so ids are per document", () => {
+        const first = new BookmarkIds();
+        first.getId("a");
+        first.getId("b");
+
+        const second = new BookmarkIds();
+
+        expect(second.getId("a")).to.equal(1);
+    });
+});

src/file/paragraph/links/bookmark-ids.ts

@@ -0,0 +1,48 @@
+/**
+ * Per-document numeric id allocation for bookmarks.
+ *
+ * @module
+ */
+
+/**
+ * Allocates the numeric ids written to `w:bookmarkStart` and `w:bookmarkEnd`.
+ *
+ * Ids must be unique within a document, and a bookmark's start and end must
+ * share one. Both markers look up their bookmark name here, so whichever is
+ * serialized first allocates and the other reuses.
+ *
+ * @example
+ * ```typescript
+ * const ids = new BookmarkIds();
+ * ids.getId("intro"); // 1
+ * ids.getId("summary"); // 2
+ * ids.getId("intro"); // 1
+ * ```
+ */
+export class BookmarkIds {
+    // eslint-disable-next-line functional/prefer-readonly-type
+    private readonly ids: Map<string, number>;
+
+    public constructor() {
+        this.ids = new Map<string, number>();
+    }
+
+    /**
+     * Returns the id for a bookmark name, allocating one on first use.
+     *
+     * @returns The id shared by that bookmark's start and end markers
+     */
+    public getId(name: string): number {
+        const existing = this.ids.get(name);
+
+        if (existing !== undefined) {
+            return existing;
+        }
+
+        const id = this.ids.size + 1;
+        // eslint-disable-next-line functional/immutable-data
+        this.ids.set(name, id);
+
+        return id;
+    }
+}

src/file/paragraph/links/bookmark.spec.ts

@@ -1,41 +1,84 @@
-import { assert, beforeEach, describe, expect, it } from "vitest";
+import { beforeEach, describe, expect, it } from "vitest";
 
-import { Utility } from "tests/utility";
+import { Formatter } from "@export/formatter";
+import type { IContext } from "@file/xml-components";
 
+import type { IViewWrapper } from "../../document-wrapper";
+import type { File } from "../../file";
 import { TextRun } from "../run";
-import { Bookmark } from "./bookmark";
+import { Bookmark, BookmarkStart } from "./bookmark";
+import { BookmarkIds } from "./bookmark-ids";
+
+const documentContext = (): IContext => ({
+    file: { BookmarkIds: new BookmarkIds() } as unknown as File,
+    viewWrapper: {} as unknown as IViewWrapper,
+    stack: [],
+});
 
 describe("Bookmark", () => {
+    let context: IContext;
+    let textRun: TextRun;
     let bookmark: Bookmark;
 
     beforeEach(() => {
-        bookmark = new Bookmark({
-            id: "anchor",
-            children: [new TextRun("Internal Link")],
-        });
+        context = documentContext();
+        textRun = new TextRun("Internal Link");
+        bookmark = new Bookmark({ id: "anchor", children: [textRun] });
     });
 
     it("should create a bookmark with three root elements", () => {
-        const newJson = Utility.jsonify(bookmark);
-        assert.equal(newJson.rootKey, undefined);
-        assert.equal(newJson.start.rootKey, "w:bookmarkStart");
-        assert.equal(newJson.children[0].rootKey, "w:r");
-        assert.equal(newJson.end.rootKey, "w:bookmarkEnd");
+        expect(new Formatter().format(bookmark.start, context)).to.have.property("w:bookmarkStart");
+        expect(new Formatter().format(textRun, context)).to.have.property("w:r");
+        expect(new Formatter().format(bookmark.end, context)).to.have.property("w:bookmarkEnd");
     });
 
     it("should create a bookmark with the correct attributes on the bookmark start element", () => {
-        const newJson = Utility.jsonify(bookmark);
+        const tree = new Formatter().format(bookmark.start, context);
 
-        assert.equal(newJson.start.root[0].root.name, "anchor");
+        expect(tree["w:bookmarkStart"]._attr["w:name"]).to.equal("anchor");
     });
 
-    it("should create a bookmark with the correct attributes on the text element", () => {
-        const newJson = Utility.jsonify(bookmark);
-        assert.equal(JSON.stringify(newJson.children[0].root[1].root[1]), JSON.stringify("Internal Link"));
+    it("should keep the bookmark's children", () => {
+        expect(bookmark.children).to.deep.equal([textRun]);
+        expect(JSON.stringify(new Formatter().format(textRun, context))).to.contain("Internal Link");
     });
 
     it("should create a bookmark with the correct attributes on the bookmark end element", () => {
-        const newJson = Utility.jsonify(bookmark);
-        expect(newJson.end.root[0].root.id).to.be.a("number");
+        const tree = new Formatter().format(bookmark.end, context);
+
+        expect(tree["w:bookmarkEnd"]._attr["w:id"]).to.be.a("number");
+    });
+
+    it("should pair the start and end elements with the same id", () => {
+        const start = new Formatter().format(bookmark.start, context);
+        const end = new Formatter().format(bookmark.end, context);
+
+        expect(start["w:bookmarkStart"]._attr["w:id"]).to.equal(end["w:bookmarkEnd"]._attr["w:id"]);
+    });
+
+    // Regression: a per-instance generator gave every bookmark `w:id="1"`, so
+    // start and end pairing was ambiguous and Word could not resolve references.
+    it("should give each bookmark in a document a distinct id", () => {
+        const bookmarks = ["first", "second", "third"].map((id) => new Bookmark({ id, children: [new TextRun(id)] }));
+
+        const ids = bookmarks.map((item) => new Formatter().format(item.start, context)["w:bookmarkStart"]._attr["w:id"]);
+
+        expect(ids).to.deep.equal([1, 2, 3]);
+    });
+
+    it("should number bookmarks from one in every document", () => {
+        const first = new Bookmark({ id: "a", children: [new TextRun("a")] });
+        new Formatter().format(first.start, context);
+
+        const otherDocument = documentContext();
+        const second = new Bookmark({ id: "b", children: [new TextRun("b")] });
+
+        expect(new Formatter().format(second.start, otherDocument)["w:bookmarkStart"]._attr["w:id"]).to.equal(1);
+    });
+
+    it("should keep an explicitly supplied id", () => {
+        const tree = new Formatter().format(new BookmarkStart("named", 7), context);
+
+        expect(tree["w:bookmarkStart"]._attr["w:id"]).to.equal(7);
     });
 });

src/file/paragraph/links/bookmark.ts

@@ -8,8 +8,7 @@
  *
  * @module
  */
-import { XmlComponent } from "@file/xml-components";
-import { bookmarkUniqueNumericIdGen } from "@util/convenience-functions";
+import { type IContext, type IXmlableObject, XmlComponent } from "@file/xml-components";
 
 import type { ParagraphChild } from "../paragraph";
 import { BookmarkEndAttributes, BookmarkStartAttributes } from "./bookmark-attributes";
@@ -68,18 +67,14 @@ export type IBookmarkOptions = {
  * ```
  */
 export class Bookmark {
-    private readonly bookmarkUniqueNumericId = bookmarkUniqueNumericIdGen();
-
     public readonly start: BookmarkStart;
     public readonly children: readonly ParagraphChild[];
     public readonly end: BookmarkEnd;
 
     public constructor(options: IBookmarkOptions) {
-        const linkId = this.bookmarkUniqueNumericId();
-
-        this.start = new BookmarkStart(options.id, linkId);
+        this.start = new BookmarkStart(options.id);
         this.children = options.children;
-        this.end = new BookmarkEnd(linkId);
+        this.end = new BookmarkEnd(options.id);
     }
 }
 
@@ -104,20 +99,34 @@ export class Bookmark {
  * </xsd:complexType>
  * ```
  *
+ * Without `linkId`, the id is resolved from the document on serialization.
+ *
  * @example
  * ```typescript
- * new BookmarkStart("myBookmark", 1);
+ * new BookmarkStart("myBookmark");
+ * new BookmarkStart("myBookmark", 1); // explicit id
  * ```
  */
 export class BookmarkStart extends XmlComponent {
-    public constructor(id: string, linkId: number) {
+    private readonly name: string;
+    private readonly linkId?: number;
+
+    public constructor(id: string, linkId?: number) {
         super("w:bookmarkStart");
 
-        const attributes = new BookmarkStartAttributes({
-            name: id,
-            id: linkId,
-        });
-        this.root.push(attributes);
+        this.name = id;
+        this.linkId = linkId;
+    }
+
+    public prepForXml(context: IContext): IXmlableObject | undefined {
+        this.root.push(
+            new BookmarkStartAttributes({
+                name: this.name,
+                id: this.linkId ?? context.file.BookmarkIds.getId(this.name),
+            }),
+        );
+
+        return super.prepForXml(context);
     }
 }
 
@@ -142,18 +151,31 @@ export class BookmarkStart extends XmlComponent {
  * </xsd:complexType>
  * ```
  *
+ * Given a name, the id is resolved from the document on serialization, which is
+ * how it matches the `BookmarkStart` for that name.
+ *
  * @example
  * ```typescript
- * new BookmarkEnd(1);
+ * new BookmarkEnd("myBookmark");
+ * new BookmarkEnd(1); // explicit id
  * ```
  */
 export class BookmarkEnd extends XmlComponent {
-    public constructor(linkId: number) {
+    private readonly nameOrLinkId: string | number;
+
+    public constructor(nameOrLinkId: string | number) {
         super("w:bookmarkEnd");
 
-        const attributes = new BookmarkEndAttributes({
-            id: linkId,
-        });
-        this.root.push(attributes);
+        this.nameOrLinkId = nameOrLinkId;
+    }
+
+    public prepForXml(context: IContext): IXmlableObject | undefined {
+        this.root.push(
+            new BookmarkEndAttributes({
+                id: typeof this.nameOrLinkId === "number" ? this.nameOrLinkId : context.file.BookmarkIds.getId(this.nameOrLinkId),
+            }),
+        );
+
+        return super.prepForXml(context);
     }
 }

src/file/paragraph/paragraph.spec.ts

@@ -12,6 +12,7 @@ import { ShadingType } from "../shading";
 import { AlignmentType, HeadingLevel, LeaderType, PageBreak, TabStopPosition, TabStopType } from "./formatting";
 import { FrameAnchorType } from "./frame";
 import { Bookmark, ExternalHyperlink } from "./links";
+import { BookmarkIds } from "./links/bookmark-ids";
 import { Paragraph } from "./paragraph";
 import { TextRun } from "./run";
 
@@ -717,13 +718,18 @@ describe("Paragraph", () => {
                 }),
             ],
         });
-        const tree = new Formatter().format(paragraph);
+        const tree = new Formatter().format(paragraph, {
+            file: { BookmarkIds: new BookmarkIds() } as unknown as File,
+            viewWrapper: {} as unknown as IViewWrapper,
+            stack: [],
+        });
+
         expect(tree).to.deep.equal({
             "w:p": [
                 {
                     "w:bookmarkStart": {
                         _attr: {
-                            "w:id": -101,
+                            "w:id": 1,
                             "w:name": "test-id",
                         },
                     },
@@ -745,7 +751,7 @@ describe("Paragraph", () => {
                 {
                     "w:bookmarkEnd": {
                         _attr: {
-                            "w:id": -101,
+                            "w:id": 1,
                         },
                     },
                 },