From aab0f917406bf78117829c449ab9f12bf0704134 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 19 Dec 2019 16:37:03 +0100 Subject: [PATCH 1/5] [api-minor] Simplify the *fallback* fake worker loader code in `src/display/api.js` For performance reasons, and to avoid hanging the browser UI, the PDF.js library should *always* be used with web workers enabled. At this point in time all of the supported browsers should have proper worker support, and Node.js is thus the only environment where workers aren't supported. Hence it no longer seems relevant/necessary to provide, by default, fake worker loaders for various JS builders/bundlers/frameworks in the PDF.js code itself.[1] In order to simplify things, the fake worker loader code is thus simplified to now *only* support Node.js usage respectively "normal" browser usage out-of-the-box.[2] *Please note:* The officially intended way of using the PDF.js library is with workers enabled, which can be done by setting `GlobalWorkerOptions.workerSrc`, `GlobalWorkerOptions.workerPort`, or manually providing a `PDFWorker` instance when calling `getDocument`. --- [1] Note that it's still possible to *manually* disable workers, simply my manually loading the built `pdf.worker.js` file into the (current) global scope, however this's mostly intended for testing/debugging purposes. [2] Unfortunately some bundlers such as Webpack, when used with third-party deployments of the PDF.js library, will start to print `Critical dependency: ...` warnings when run against the built `pdf.js` file from this patch. The reason is that despite the `require` calls being protected by *runtime* `isNodeJS` checks, it's not possible to simply tell Webpack to just ignore the `require`; please see [Webpack issue 8826](https://github.com/webpack/webpack) and libraries such as [require-fool-webpack](https://github.com/sindresorhus/require-fool-webpack). --- examples/browserify/main.js | 2 +- examples/webpack/main.js | 2 +- gulpfile.js | 2 -- package-lock.json | 6 ----- package.json | 1 - src/display/api.js | 50 ++++++++----------------------------- 6 files changed, 13 insertions(+), 50 deletions(-) diff --git a/examples/browserify/main.js b/examples/browserify/main.js index 6c4e144b1..8554ffc9b 100644 --- a/examples/browserify/main.js +++ b/examples/browserify/main.js @@ -5,7 +5,7 @@ var pdfjsLib = require('pdfjs-dist'); -var pdfPath = '../helloworld/helloworld.pdf'; +var pdfPath = '../learning/helloworld.pdf'; // Setting worker path to worker bundle. pdfjsLib.GlobalWorkerOptions.workerSrc = diff --git a/examples/webpack/main.js b/examples/webpack/main.js index f8bfd179b..bc21b5f32 100644 --- a/examples/webpack/main.js +++ b/examples/webpack/main.js @@ -5,7 +5,7 @@ var pdfjsLib = require('pdfjs-dist'); -var pdfPath = '../helloworld/helloworld.pdf'; +var pdfPath = '../learning/helloworld.pdf'; // Setting worker path to worker bundle. pdfjsLib.GlobalWorkerOptions.workerSrc = diff --git a/gulpfile.js b/gulpfile.js index 6558a6618..bbe9fa7d6 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -1320,7 +1320,6 @@ gulp.task('dist-pre', gulp.series('generic', 'components', 'image_decoders', bugs: DIST_BUGS_URL, license: DIST_LICENSE, dependencies: { - 'node-ensure': '^0.0.0', // shim for node for require.ensure 'worker-loader': '^2.0.0', // used in external/dist/webpack.json }, peerDependencies: { @@ -1330,7 +1329,6 @@ gulp.task('dist-pre', gulp.series('generic', 'components', 'image_decoders', 'fs': false, 'http': false, 'https': false, - 'node-ensure': false, 'url': false, 'zlib': false, }, diff --git a/package-lock.json b/package-lock.json index 9442257c2..78c1bb848 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6682,12 +6682,6 @@ "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", "dev": true }, - "node-ensure": { - "version": "0.0.0", - "resolved": "https://registry.npmjs.org/node-ensure/-/node-ensure-0.0.0.tgz", - "integrity": "sha1-7K52QVDemYYexcgQ/V0Jaxg5Mqc=", - "dev": true - }, "node-libs-browser": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/node-libs-browser/-/node-libs-browser-2.1.0.tgz", diff --git a/package.json b/package.json index 87bce96c0..46fdf0801 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,6 @@ "jstransformer-markdown-it": "^2.1.0", "merge-stream": "^1.0.1", "mkdirp": "^0.5.1", - "node-ensure": "^0.0.0", "prettier": "^1.19.1", "rimraf": "^2.7.1", "streamqueue": "^1.1.2", diff --git a/src/display/api.js b/src/display/api.js index b83292872..e26265e72 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -12,7 +12,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/* globals requirejs, __non_webpack_require__ */ +/* globals __non_webpack_require__ */ /* eslint no-var: error */ /** @@ -34,6 +34,7 @@ import { FontFaceObject, FontLoader } from './font_loader'; import { apiCompatibilityParams } from './api_compatibility'; import { CanvasGraphics } from './canvas'; import { GlobalWorkerOptions } from './worker_options'; +import { isNodeJS } from '../shared/is_node'; import { MessageHandler } from '../shared/message_handler'; import { Metadata } from './metadata'; import { PDFDataTransportStream } from './transport_stream'; @@ -47,28 +48,12 @@ let fallbackWorkerSrc; let fakeWorkerFilesLoader = null; if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) { - let useRequireEnsure = false; - // For GENERIC build we need to add support for different fake file loaders - // for different frameworks. - if (typeof window === 'undefined') { - // node.js - disable worker and set require.ensure. + if (isNodeJS && typeof __non_webpack_require__ === 'function') { + // Workers aren't supported in Node.js, force-disabling them there. isWorkerDisabled = true; - if (typeof __non_webpack_require__.ensure === 'undefined') { - __non_webpack_require__.ensure = __non_webpack_require__('node-ensure'); - } - useRequireEnsure = true; - } else if (typeof __non_webpack_require__ !== 'undefined' && - typeof __non_webpack_require__.ensure === 'function') { - useRequireEnsure = true; - } - if (typeof requirejs !== 'undefined' && requirejs.toUrl) { - fallbackWorkerSrc = requirejs.toUrl('pdfjs-dist/build/pdf.worker.js'); - } - const dynamicLoaderSupported = - typeof requirejs !== 'undefined' && requirejs.load; - fakeWorkerFilesLoader = useRequireEnsure ? (function() { - return new Promise(function(resolve, reject) { - __non_webpack_require__.ensure([], function() { + + fakeWorkerFilesLoader = function() { + return new Promise(function(resolve, reject) { try { let worker; if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('LIB')) { @@ -80,22 +65,9 @@ if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) { } catch (ex) { reject(ex); } - }, reject, 'pdfjsWorker'); - }); - }) : dynamicLoaderSupported ? (function() { - return new Promise(function(resolve, reject) { - requirejs(['pdfjs-dist/build/pdf.worker'], function(worker) { - try { - resolve(worker.WorkerMessageHandler); - } catch (ex) { - reject(ex); - } - }, reject); - }); - }) : null; - - if (!fallbackWorkerSrc && typeof document === 'object' && - 'currentScript' in document) { + }); + }; + } else if (typeof document === 'object' && 'currentScript' in document) { const pdfjsFilePath = document.currentScript && document.currentScript.src; if (pdfjsFilePath) { fallbackWorkerSrc = @@ -1557,7 +1529,7 @@ const PDFWorker = (function PDFWorkerClosure() { const mainWorkerMessageHandler = getMainThreadWorkerMessageHandler(); if (mainWorkerMessageHandler) { - // The worker was already loaded using a `