Skip to content

Commit

Permalink
Fix Viewer API definitions and include in CI
Browse files Browse the repository at this point in the history
The Viewer API definitions do not compile because of missing imports and
anonymous objects are typed as `Object`. These issues were not caught
during CI because the test project was not compiling anything from the
Viewer API.

As an example of the first problem:

```
/**
 * @implements MyInterface
 */
export class MyClass {
    ...
}
```

will generate a broken definition that doesn’t import MyInterface:

```
/**
 * @implements MyInterface
 */
export class MyClass implements MyInterface {
    ...
}
```

This can be fixed by adding a typedef jsdoc to specify the import:

```
/** @typedef {import("./otherFile").MyInterface} MyInterface */
```

See jsdoc/jsdoc#1537 and
microsoft/TypeScript#22160 for more details.

As an example of the second problem:

```
/**
 * Gets the size of the specified page, converted from PDF units to inches.
 * @param {Object} An Object containing the properties: {Array} `view`,
 *   {number} `userUnit`, and {number} `rotate`.
 */
function getPageSizeInches({ view, userUnit, rotate }) {
    ...
}
```

generates the broken definition:

```
function getPageSizeInches({ view, userUnit, rotate }: Object) {
    ...
}
```

The jsdoc should specify the type of each nested property:

```
/**
 * Gets the size of the specified page, converted from PDF units to inches.
 * @param {Object} options An object containing the properties: {Array} `view`,
 *   {number} `userUnit`, and {number} `rotate`.
 * @param {number[]} options.view
 * @param {number} options.userUnit
 * @param {number} options.rotate
 */
```
  • Loading branch information
michael-yx-wu authored and rousek committed Aug 10, 2022
1 parent 330e6ed commit 33214c6
Show file tree
Hide file tree
Showing 15 changed files with 47 additions and 11 deletions.
2 changes: 1 addition & 1 deletion external/dist/legacy/web/pdf_viewer.d.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "pdfjs-dist/types/web/pdf_viewer.component";
export * from "../../types/web/pdf_viewer.component";
2 changes: 1 addition & 1 deletion external/dist/web/pdf_viewer.d.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "pdfjs-dist/types/web/pdf_viewer.component";
export * from "../types/web/pdf_viewer.component";
3 changes: 3 additions & 0 deletions test/types/main.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { getDocument } from "pdfjs-dist";
import { EventBus } from "pdfjs-dist/web/pdf_viewer.component";

class MainTest {
eventBus: EventBus;
task: ReturnType<typeof getDocument> | undefined;

constructor(public file: string) {
this.eventBus = new EventBus({});
}

loadPdf() {
Expand Down
2 changes: 1 addition & 1 deletion test/types/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
],
"paths": {
"pdfjs-dist": ["../../build/typestest"],
"pdfjs-dist/*": ["../../build/typestest/build/*"]
"pdfjs-dist/*": ["../../build/typestest/types/*"]
}
},
"files": [
Expand Down
3 changes: 3 additions & 0 deletions web/annotation_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
* limitations under the License.
*/

// eslint-disable-next-line max-len
/** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */

import { AnnotationLayer } from "pdfjs-lib";
import { NullL10n } from "./l10n_utils.js";
import { SimpleLinkService } from "./pdf_link_service.js";
Expand Down
1 change: 0 additions & 1 deletion web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ function isSameScale(oldScale, newScale) {

/**
* Simple viewer control to display PDF content/pages.
* @implements {IRenderableView}
*/
class BaseViewer {
/**
Expand Down
7 changes: 5 additions & 2 deletions web/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ class IPDFLinkService {
* @interface
*/
class IRenderableView {
constructor() {
/** @type {function | null} */
this.resume = null;
}

/**
* @type {string} - Unique ID for rendering queue.
*/
Expand All @@ -120,8 +125,6 @@ class IRenderableView {
* @returns {Promise} Resolved on draw completion.
*/
draw() {}

resume() {}
}

/**
Expand Down
2 changes: 2 additions & 0 deletions web/pdf_link_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* limitations under the License.
*/

/** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */

import { parseQueryString } from "./ui_utils.js";

/**
Expand Down
2 changes: 2 additions & 0 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* limitations under the License.
*/

/** @typedef {import("./interfaces").IRenderableView} IRenderableView */

import {
AnnotationMode,
createPromiseCapability,
Expand Down
1 change: 1 addition & 0 deletions web/pdf_rendering_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class PDFRenderingQueue {
this.pdfThumbnailViewer = null;
this.onIdle = null;
this.highestPriorityPage = null;
/** @type {number} */
this.idleTimeout = null;
this.printing = false;
this.isThumbnailViewEnabled = false;
Expand Down
3 changes: 3 additions & 0 deletions web/struct_tree_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
* limitations under the License.
*/

// eslint-disable-next-line max-len
/** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */

const PDF_ROLE_TO_HTML_ROLE = {
// Document level structure types
Document: null, // There's a "document" role, but it doesn't make sense here.
Expand Down
5 changes: 4 additions & 1 deletion web/text_highlighter.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/

/**
* @typedef {Object} TextHighlighter
* @typedef {Object} TextHighlighterOptions
* @property {PDFFindController} findController
* @property {EventBus} eventBus - The application event bus.
* @property {number} pageIndex - The page index.
Expand All @@ -25,6 +25,9 @@
* either the text layer or XFA layer depending on the type of document.
*/
class TextHighlighter {
/**
* @param {TextHighlighterOptions} options
*/
constructor({ findController, eventBus, pageIndex }) {
this.findController = findController;
this.matches = [];
Expand Down
3 changes: 3 additions & 0 deletions web/text_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
* limitations under the License.
*/

// eslint-disable-next-line max-len
/** @typedef {import("./interfaces").IPDFTextLayerFactory} IPDFTextLayerFactory */

import { renderTextLayer } from "pdfjs-lib";

const EXPAND_DIVS_TIMEOUT = 300; // ms
Expand Down
19 changes: 15 additions & 4 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,12 +284,23 @@ function roundToDivide(x, div) {
return r === 0 ? x : Math.round(x - r + div);
}

/**
* @typedef {Object} GetPageSizeInchesParameters
* @property {number[]} view
* @property {number} userUnit
* @property {number} rotate
*/

/**
* @typedef {Object} PageSize
* @property {number} width - In inches.
* @property {number} height - In inches.
*/

/**
* Gets the size of the specified page, converted from PDF units to inches.
* @param {Object} An Object containing the properties: {Array} `view`,
* {number} `userUnit`, and {number} `rotate`.
* @returns {Object} An Object containing the properties: {number} `width`
* and {number} `height`, given in inches.
* @param {GetPageSizeInchesParameters} params
* @returns {PageSize}
*/
function getPageSizeInches({ view, userUnit, rotate }) {
const [x1, y1, x2, y2] = view;
Expand Down
3 changes: 3 additions & 0 deletions web/xfa_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@
* limitations under the License.
*/

/** @typedef {import("./interfaces").IPDFXfaLayerFactory} IPDFXfaLayerFactory */

import { XfaLayer } from "pdfjs-lib";

/**
* @typedef {Object} XfaLayerBuilderOptions
* @property {HTMLDivElement} pageDiv
* @property {PDFPage} pdfPage
* @property {Object} [xfaHtml]
* @property {AnnotationStorage} [annotationStorage]
*/

Expand Down

0 comments on commit 33214c6

Please sign in to comment.