Skip to content

Fix the gulp types task to run on Windows, and place the TypeScript definitions correctly in pdfjs-dist #12168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

  • Fix the gulp types task to run on Windows. Currently this fails, and the solution was to "borrow" the same formatting as used in the gulp jsdoc task.

  • Place the TypeScript definitions in their own types directory, when building pdfjs-dist. These should not be cluttering the main build directory, especially since the generated TypeScript definitions consists of multiple folders. (Only if the TypeScript definitions would be concatenated into a single file, would placing them directly in pdfjs-dist/build be acceptable.)

@tamuratak
Copy link
Contributor

You should change the types entry in the generated package.json in pdfjs-dist to point to the pdf.d.ts file.

diff --git a/gulpfile.js b/gulpfile.js
index 7da61ed1..6f3d480a 100644
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -1640,7 +1640,7 @@ function packageBowerJson() {
     name: DIST_NAME,
     version: VERSION,
     main: "build/pdf.js",
-    types: "build/pdf.d.ts",
+    types: "types/pdf.d.ts",
     description: DIST_DESCRIPTION,
     keywords: DIST_KEYWORDS,
     homepage: DIST_HOMEPAGE,

See https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

@tamuratak
Copy link
Contributor

The following fixes typestest:

diff --git a/gulpfile.js b/gulpfile.js
index e44d6e04..51b9921f 100644
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -1374,7 +1374,7 @@ gulp.task(
           .pipe(gulp.dest(TYPESTEST_DIR + "build/")),
         gulp
           .src(TYPES_BUILD_DIR + "**/**")
-          .pipe(gulp.dest(TYPESTEST_DIR + "build/")),
+          .pipe(gulp.dest(TYPESTEST_DIR + "types/")),
       ]);
     },
     function (done) {

I think typestest should directly run a test against files inbuild/dist or node_modules/pdfjs-dist, instead of copying files.

… definitions correctly in `pdfjs-dist`

 - Fix the `gulp types` task to run on Windows. Currently this fails, and the solution was to "borrow" the same formatting as used in the `gulp jsdoc` task.

 - Place the TypeScript definitions in their own `types` directory, when building `pdfjs-dist`. These should *not* be cluttering the main `build` directory, especially since the generated TypeScript definitions consists of *multiple folders*. (Only if the TypeScript definitions would be concatenated into *a single file*, would placing them directly in `pdfjs-dist/build` be acceptable.)
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2020

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/578d0e53c9437a2/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2020

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/578d0e53c9437a2/output.txt

Total script time: 3.49 mins

Published

@timvandermeij timvandermeij merged commit 63e33a5 into mozilla:master Aug 4, 2020
@timvandermeij
Copy link
Contributor

Tested locally as well and this seems to work just fine. Thanks!

@Snuffleupagus Snuffleupagus deleted the fix-types branch August 4, 2020 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants