From a0437381e71ca555b0d53a503551209395a31674 Mon Sep 17 00:00:00 2001 From: Maxim Fedorenko Date: Fri, 28 Apr 2023 13:17:02 +0300 Subject: [PATCH 1/3] fix: do not use static numeric counters --- src/file/drawing/anchor/anchor.spec.ts | 8 -------- src/file/drawing/doc-properties/doc-properties.ts | 6 ++++-- src/file/drawing/drawing.spec.ts | 8 -------- src/file/numbering/numbering.spec.ts | 10 ---------- src/file/numbering/numbering.ts | 10 ++++++---- src/file/paragraph/links/bookmark.ts | 6 ++++-- src/file/paragraph/paragraph.spec.ts | 4 ++-- src/file/paragraph/run/image-run.spec.ts | 2 -- src/util/convenience-functions.ts | 8 ++++---- 9 files changed, 20 insertions(+), 42 deletions(-) diff --git a/src/file/drawing/anchor/anchor.spec.ts b/src/file/drawing/anchor/anchor.spec.ts index b6a368b4e6..50b997bb7b 100644 --- a/src/file/drawing/anchor/anchor.spec.ts +++ b/src/file/drawing/anchor/anchor.spec.ts @@ -40,14 +40,6 @@ const createAnchor = (drawingOptions: IDrawingOptions): Anchor => ); describe("Anchor", () => { - before(() => { - stub(convenienceFunctions, "docPropertiesUniqueNumericId").callsFake(() => 0); - }); - - after(() => { - (convenienceFunctions.docPropertiesUniqueNumericId as SinonStub).restore(); - }); - let anchor: Anchor; describe("#constructor()", () => { diff --git a/src/file/drawing/doc-properties/doc-properties.ts b/src/file/drawing/doc-properties/doc-properties.ts index 8b0093c7f1..5df1497bb5 100644 --- a/src/file/drawing/doc-properties/doc-properties.ts +++ b/src/file/drawing/doc-properties/doc-properties.ts @@ -2,7 +2,7 @@ import { IContext, IXmlableObject, NextAttributeComponent, XmlComponent } from "@file/xml-components"; import { ConcreteHyperlink } from "@file/paragraph"; -import { docPropertiesUniqueNumericId } from "@util/convenience-functions"; +import { docPropertiesUniqueNumericIdGen } from "@util/convenience-functions"; import { createHyperlinkClick } from "./doc-properties-children"; @@ -25,6 +25,8 @@ export interface DocPropertiesOptions { } export class DocProperties extends XmlComponent { + private readonly docPropertiesUniqueNumericId = docPropertiesUniqueNumericIdGen(); + public constructor({ name, description, title }: DocPropertiesOptions = { name: "", description: "", title: "" }) { super("wp:docPr"); @@ -32,7 +34,7 @@ export class DocProperties extends XmlComponent { new NextAttributeComponent({ id: { key: "id", - value: docPropertiesUniqueNumericId(), + value: this.docPropertiesUniqueNumericId(), }, name: { key: "name", diff --git a/src/file/drawing/drawing.spec.ts b/src/file/drawing/drawing.spec.ts index 8850881159..fc9a35b4b4 100644 --- a/src/file/drawing/drawing.spec.ts +++ b/src/file/drawing/drawing.spec.ts @@ -30,14 +30,6 @@ const createDrawing = (drawingOptions?: IDrawingOptions): Drawing => ); describe("Drawing", () => { - before(() => { - stub(convenienceFunctions, "docPropertiesUniqueNumericId").callsFake(() => 0); - }); - - after(() => { - (convenienceFunctions.docPropertiesUniqueNumericId as SinonStub).restore(); - }); - let currentBreak: Drawing; describe("#constructor()", () => { diff --git a/src/file/numbering/numbering.spec.ts b/src/file/numbering/numbering.spec.ts index ff4c781ebe..4af0286b07 100644 --- a/src/file/numbering/numbering.spec.ts +++ b/src/file/numbering/numbering.spec.ts @@ -7,16 +7,6 @@ import * as convenienceFunctions from "@util/convenience-functions"; import { Numbering } from "./numbering"; describe("Numbering", () => { - before(() => { - stub(convenienceFunctions, "abstractNumUniqueNumericId").callsFake(() => 0); - stub(convenienceFunctions, "concreteNumUniqueNumericId").callsFake(() => 0); - }); - - after(() => { - (convenienceFunctions.abstractNumUniqueNumericId as SinonStub).restore(); - (convenienceFunctions.concreteNumUniqueNumericId as SinonStub).restore(); - }); - describe("#constructor", () => { it("creates a default numbering with one abstract and one concrete instance", () => { const numbering = new Numbering({ diff --git a/src/file/numbering/numbering.ts b/src/file/numbering/numbering.ts index 4ec5206327..4a73c01a87 100644 --- a/src/file/numbering/numbering.ts +++ b/src/file/numbering/numbering.ts @@ -2,7 +2,7 @@ // https://stackoverflow.com/questions/58622437/purpose-of-abstractnum-and-numberinginstance import { AlignmentType } from "@file/paragraph"; import { IContext, IXmlableObject, XmlComponent } from "@file/xml-components"; -import { abstractNumUniqueNumericId, concreteNumUniqueNumericId, convertInchesToTwip } from "@util/convenience-functions"; +import { abstractNumUniqueNumericIdGen, concreteNumUniqueNumericIdGen, convertInchesToTwip } from "@util/convenience-functions"; import { DocumentAttributes } from "../document/document-attributes"; import { AbstractNumbering } from "./abstract-numbering"; @@ -30,6 +30,8 @@ export class Numbering extends XmlComponent { private readonly abstractNumberingMap = new Map(); private readonly concreteNumberingMap = new Map(); private readonly referenceConfigMap = new Map(); + private readonly abstractNumUniqueNumericId = abstractNumUniqueNumericIdGen(); + private readonly concreteNumUniqueNumericId = concreteNumUniqueNumericIdGen(); public constructor(options: INumberingOptions) { super("w:numbering"); @@ -55,7 +57,7 @@ export class Numbering extends XmlComponent { }), ); - const abstractNumbering = new AbstractNumbering(abstractNumUniqueNumericId(), [ + const abstractNumbering = new AbstractNumbering(this.abstractNumUniqueNumericId(), [ { level: 0, format: LevelFormat.BULLET, @@ -176,7 +178,7 @@ export class Numbering extends XmlComponent { this.abstractNumberingMap.set("default-bullet-numbering", abstractNumbering); for (const con of options.config) { - this.abstractNumberingMap.set(con.reference, new AbstractNumbering(abstractNumUniqueNumericId(), con.levels)); + this.abstractNumberingMap.set(con.reference, new AbstractNumbering(this.abstractNumUniqueNumericId(), con.levels)); this.referenceConfigMap.set(con.reference, con.levels); } } @@ -209,7 +211,7 @@ export class Numbering extends XmlComponent { const firstLevelStartNumber = referenceConfigLevels && referenceConfigLevels[0].start; const concreteNumberingSettings = { - numId: concreteNumUniqueNumericId(), + numId: this.concreteNumUniqueNumericId(), abstractNumId: abstractNumbering.id, reference, instance, diff --git a/src/file/paragraph/links/bookmark.ts b/src/file/paragraph/links/bookmark.ts index 8bb7c9b5ed..c6082b7d3d 100644 --- a/src/file/paragraph/links/bookmark.ts +++ b/src/file/paragraph/links/bookmark.ts @@ -1,17 +1,19 @@ // http://officeopenxml.com/WPbookmark.php import { XmlComponent } from "@file/xml-components"; -import { bookmarkUniqueNumericId } from "@util/convenience-functions"; +import { bookmarkUniqueNumericIdGen } from "@util/convenience-functions"; import { ParagraphChild } from "../paragraph"; import { BookmarkEndAttributes, BookmarkStartAttributes } from "./bookmark-attributes"; export class Bookmark { + private readonly bookmarkUniqueNumericId = bookmarkUniqueNumericIdGen(); + public readonly start: BookmarkStart; public readonly children: readonly ParagraphChild[]; public readonly end: BookmarkEnd; public constructor(options: { readonly id: string; readonly children: readonly ParagraphChild[] }) { - const linkId = bookmarkUniqueNumericId(); + const linkId = this.bookmarkUniqueNumericId(); this.start = new BookmarkStart(options.id, linkId); this.children = options.children; diff --git a/src/file/paragraph/paragraph.spec.ts b/src/file/paragraph/paragraph.spec.ts index 7d21b5d171..8f7ed03801 100644 --- a/src/file/paragraph/paragraph.spec.ts +++ b/src/file/paragraph/paragraph.spec.ts @@ -20,12 +20,12 @@ import { TextRun } from "./run"; describe("Paragraph", () => { before(() => { stub(convenienceFunctions, "uniqueId").callsFake(() => "test-unique-id"); - stub(convenienceFunctions, "bookmarkUniqueNumericId").callsFake(() => -101); + stub(convenienceFunctions, "bookmarkUniqueNumericIdGen").callsFake(() => () => -101); }); after(() => { (convenienceFunctions.uniqueId as SinonStub).restore(); - (convenienceFunctions.bookmarkUniqueNumericId as SinonStub).restore(); + (convenienceFunctions.bookmarkUniqueNumericIdGen as SinonStub).restore(); }); describe("#constructor()", () => { diff --git a/src/file/paragraph/run/image-run.spec.ts b/src/file/paragraph/run/image-run.spec.ts index 9e9ad89505..47e947771a 100644 --- a/src/file/paragraph/run/image-run.spec.ts +++ b/src/file/paragraph/run/image-run.spec.ts @@ -11,12 +11,10 @@ import { ImageRun } from "./image-run"; describe("ImageRun", () => { before(() => { stub(convenienceFunctions, "uniqueId").callsFake(() => "test-unique-id"); - stub(convenienceFunctions, "docPropertiesUniqueNumericId").callsFake(() => 0); }); after(() => { (convenienceFunctions.uniqueId as SinonStub).restore(); - (convenienceFunctions.docPropertiesUniqueNumericId as SinonStub).restore(); }); describe("#constructor()", () => { diff --git a/src/util/convenience-functions.ts b/src/util/convenience-functions.ts index 543b6e9c7c..c866ab5064 100644 --- a/src/util/convenience-functions.ts +++ b/src/util/convenience-functions.ts @@ -11,9 +11,9 @@ export const uniqueNumericIdCreator = (initial = 0): (() => number) => { return () => ++currentCount; }; -export const abstractNumUniqueNumericId = uniqueNumericIdCreator(); -export const concreteNumUniqueNumericId = uniqueNumericIdCreator(1); // Setting initial to 1 as we have numId = 1 for "default-bullet-numbering" -export const docPropertiesUniqueNumericId = uniqueNumericIdCreator(); -export const bookmarkUniqueNumericId = uniqueNumericIdCreator(); +export const abstractNumUniqueNumericIdGen = () => uniqueNumericIdCreator(); +export const concreteNumUniqueNumericIdGen = () => uniqueNumericIdCreator(1); // Setting initial to 1 as we have numId = 1 for "default-bullet-numbering" +export const docPropertiesUniqueNumericIdGen = () => uniqueNumericIdCreator(); +export const bookmarkUniqueNumericIdGen = () => uniqueNumericIdCreator(); export const uniqueId = (): string => nanoid().toLowerCase(); From e77a7dfdcd7bf88f0c4fc496434c0410140f6887 Mon Sep 17 00:00:00 2001 From: Maxim Fedorenko Date: Thu, 4 May 2023 12:30:31 +0300 Subject: [PATCH 2/3] tests: fixup tests and linting issues --- src/file/drawing/anchor/anchor.spec.ts | 4 +--- src/file/drawing/drawing.spec.ts | 8 +++----- src/file/numbering/numbering.spec.ts | 4 +--- src/file/paragraph/run/image-run.spec.ts | 8 ++++---- src/util/convenience-functions.ts | 16 +++++++++++----- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/file/drawing/anchor/anchor.spec.ts b/src/file/drawing/anchor/anchor.spec.ts index 50b997bb7b..64e628546e 100644 --- a/src/file/drawing/anchor/anchor.spec.ts +++ b/src/file/drawing/anchor/anchor.spec.ts @@ -1,8 +1,6 @@ import { assert, expect } from "chai"; -import { SinonStub, stub } from "sinon"; import { Formatter } from "@export/formatter"; -import * as convenienceFunctions from "@util/convenience-functions"; import { Utility } from "tests/utility"; @@ -458,7 +456,7 @@ describe("Anchor", () => { "wp:docPr": { _attr: { descr: "test", - id: 0, + id: 1, name: "test", title: "test", }, diff --git a/src/file/drawing/drawing.spec.ts b/src/file/drawing/drawing.spec.ts index fc9a35b4b4..c36f3e77ec 100644 --- a/src/file/drawing/drawing.spec.ts +++ b/src/file/drawing/drawing.spec.ts @@ -1,9 +1,7 @@ import { expect } from "chai"; -import { SinonStub, stub } from "sinon"; import { IContext } from "@file/xml-components"; import { Formatter } from "@export/formatter"; -import * as convenienceFunctions from "@util/convenience-functions"; import { ConcreteHyperlink, TextRun } from "../"; import { Drawing, IDrawingOptions } from "./drawing"; @@ -70,7 +68,7 @@ describe("Drawing", () => { "wp:docPr": { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, @@ -301,7 +299,7 @@ describe("Drawing", () => { "wp:docPr": { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, @@ -535,7 +533,7 @@ describe("Drawing", () => { { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, diff --git a/src/file/numbering/numbering.spec.ts b/src/file/numbering/numbering.spec.ts index 4af0286b07..a7013ef417 100644 --- a/src/file/numbering/numbering.spec.ts +++ b/src/file/numbering/numbering.spec.ts @@ -1,8 +1,6 @@ import { expect } from "chai"; -import { SinonStub, stub } from "sinon"; import { Formatter } from "@export/formatter"; -import * as convenienceFunctions from "@util/convenience-functions"; import { Numbering } from "./numbering"; @@ -18,7 +16,7 @@ describe("Numbering", () => { const abstractNums = tree["w:numbering"].filter((el) => el["w:abstractNum"]); expect(abstractNums).to.have.lengthOf(1); expect(abstractNums[0]["w:abstractNum"]).to.deep.include.members([ - { _attr: { "w:abstractNumId": 0, "w15:restartNumberingAfterBreak": 0 } }, + { _attr: { "w:abstractNumId": 1, "w15:restartNumberingAfterBreak": 0 } }, { "w:multiLevelType": { _attr: { "w:val": "hybridMultilevel" } } }, ]); diff --git a/src/file/paragraph/run/image-run.spec.ts b/src/file/paragraph/run/image-run.spec.ts index 47e947771a..2dc3cd2557 100644 --- a/src/file/paragraph/run/image-run.spec.ts +++ b/src/file/paragraph/run/image-run.spec.ts @@ -124,7 +124,7 @@ describe("ImageRun", () => { "wp:docPr": { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, @@ -376,7 +376,7 @@ describe("ImageRun", () => { "wp:docPr": { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, @@ -632,7 +632,7 @@ describe("ImageRun", () => { "wp:docPr": { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, @@ -891,7 +891,7 @@ describe("ImageRun", () => { "wp:docPr": { _attr: { descr: "", - id: 0, + id: 1, name: "", title: "", }, diff --git a/src/util/convenience-functions.ts b/src/util/convenience-functions.ts index c866ab5064..b6683b0d15 100644 --- a/src/util/convenience-functions.ts +++ b/src/util/convenience-functions.ts @@ -5,15 +5,21 @@ export const convertMillimetersToTwip = (millimeters: number): number => Math.fl export const convertInchesToTwip = (inches: number): number => Math.floor(inches * 72 * 20); -export const uniqueNumericIdCreator = (initial = 0): (() => number) => { +export type UniqueNumericIdCreator = () => number; + +export const uniqueNumericIdCreator = (initial = 0): UniqueNumericIdCreator => { let currentCount = initial; return () => ++currentCount; }; -export const abstractNumUniqueNumericIdGen = () => uniqueNumericIdCreator(); -export const concreteNumUniqueNumericIdGen = () => uniqueNumericIdCreator(1); // Setting initial to 1 as we have numId = 1 for "default-bullet-numbering" -export const docPropertiesUniqueNumericIdGen = () => uniqueNumericIdCreator(); -export const bookmarkUniqueNumericIdGen = () => uniqueNumericIdCreator(); +export const abstractNumUniqueNumericIdGen = (): UniqueNumericIdCreator => uniqueNumericIdCreator(); + +// Setting initial to 1 as we have numId = 1 for "default-bullet-numbering" +export const concreteNumUniqueNumericIdGen = (): UniqueNumericIdCreator => uniqueNumericIdCreator(1); + +export const docPropertiesUniqueNumericIdGen = (): UniqueNumericIdCreator => uniqueNumericIdCreator(); + +export const bookmarkUniqueNumericIdGen = (): UniqueNumericIdCreator => uniqueNumericIdCreator(); export const uniqueId = (): string => nanoid().toLowerCase(); From b59fa6bb16946f0af2e70894e4143b9d89a887af Mon Sep 17 00:00:00 2001 From: Dolan Miu Date: Thu, 4 May 2023 19:35:01 +0100 Subject: [PATCH 3/3] Version bump --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9d7bc23753..e7c6128e13 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "docx", - "version": "8.0.3", + "version": "8.0.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "docx", - "version": "8.0.3", + "version": "8.0.4", "license": "MIT", "dependencies": { "@types/node": "^18.0.0", diff --git a/package.json b/package.json index 7f437a5e86..bcf5418cff 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "docx", - "version": "8.0.3", + "version": "8.0.4", "description": "Easily generate .docx files with JS/TS with a nice declarative API. Works for Node and on the Browser.", "main": "build/index.js", "scripts": {