Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[api-minor] Simplify the *fallback* fake worker loader code in src/display/api.js #11418

Merged
merged 5 commits into from
Dec 21, 2019
Merged

[api-minor] Simplify the *fallback* fake worker loader code in src/display/api.js #11418

merged 5 commits into from
Dec 21, 2019

Commits on Dec 20, 2019

  1. [api-minor] Simplify the *fallback* fake worker loader code in `src/d… 

    …isplay/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).
    @Snuffleupagus
    Snuffleupagus committed Dec 20, 2019
    Configuration menu
    Copy the full SHA
    aab0f91 View commit details
    Browse the repository at this point in the history

  2. Move the fake worker loader code into the PDFWorkerClosure 

    Given that this code isn't needed "globally" in the file, it seems reasonable to move it to where it's actually used instead.
    @Snuffleupagus
    Snuffleupagus committed Dec 20, 2019
    Configuration menu
    Copy the full SHA
    591e754 View commit details
    Browse the repository at this point in the history

  3. [api-minor] Support loading the fake worker from `GlobalWorkerOptions… 

    ….workerSrc` in Node.js

There's no particularily good reason, as far as I can tell, to not support a custom worker path in Node.js environments (even if workers aren't supported). This patch thus make the Node.js fake worker loader code-path consistent with the fallback code-path used with *browser* fake worker loader.

Finally, this patch also deprecates[1] the `fallbackWorkerSrc` functionality, except in Node.js, since the user should *always* provide correct worker options since the fallback is nothing more than a best-effort solution.

---
[1] Although it probably shouldn't be removed until the next major version.
    @Snuffleupagus
    Snuffleupagus committed Dec 20, 2019
    Configuration menu
    Copy the full SHA
    a5485e1 View commit details
    Browse the repository at this point in the history

  4. Re-factor the setupFakeWorkerGlobal function (in `src/display/api.j… 

    …s`), and the `loadFakeWorker` function (in `web/app.js`)

This patch reduces some duplication, by moving *all* fake worker loader code into the `setupFakeWorkerGlobal` function. Furthermore, the functions are simplified further by using `async`/`await` where appropriate.
    @Snuffleupagus
    Snuffleupagus committed Dec 20, 2019
    Configuration menu
    Copy the full SHA
    8519f87 View commit details
    Browse the repository at this point in the history

  5. [api-minor] Tweak the Node.js fake worker loader to prevent `Critical… 

    … dependency: ...` warnings from Webpack

Since bundlers, such as Webpack, cannot be told to leave `require` statements alone we are thus forced to jump through hoops in order to prevent these warnings in third-party deployments of the PDF.js library; please see [Webpack issue 8826](https://github.com/webpack/webpack) and libraries such as [require-fool-webpack](https://github.com/sindresorhus/require-fool-webpack).

*Please note:* This is based on the assumption that code running in Node.js won't ever be affected by e.g. Content Security Policies that prevent use of `eval`. If that ever occurs, we should revert to a normal `require` statement and simply document the Webpack warnings instead.
    @Snuffleupagus
    Snuffleupagus committed Dec 20, 2019
    3 Configuration menu
    Copy the full SHA
    d370037 View commit details
    Browse the repository at this point in the history