Skip to content

Commit

Permalink
[api-minor] Tweak the Node.js fake worker loader to prevent `Critical…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
Snuffleupagus committed Dec 20, 2019
1 parent 8519f87 commit d370037
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* globals __non_webpack_require__ */
/* eslint no-var: error */

/**
Expand Down Expand Up @@ -1471,6 +1470,7 @@ const PDFWorker = (function PDFWorkerClosure() {
let fakeWorkerCapability;

if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) {
// eslint-disable-next-line no-undef
if (isNodeJS && typeof __non_webpack_require__ === 'function') {
// Workers aren't supported in Node.js, force-disabling them there.
isWorkerDisabled = true;
Expand Down Expand Up @@ -1534,8 +1534,22 @@ const PDFWorker = (function PDFWorkerClosure() {
return worker.WorkerMessageHandler;
}
if ((typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) &&
// eslint-disable-next-line no-undef
(isNodeJS && typeof __non_webpack_require__ === 'function')) {
const worker = __non_webpack_require__(getWorkerSrc());
// 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 `Critical dependency: ...` warnings in third-party
// deployments of the built `pdf.js`/`pdf.worker.js` files; see
// https://github.com/webpack/webpack/issues/8826
//
// The following hack is based on the assumption that code running in
// Node.js won't ever be affected by e.g. Content Security Policies that
// prevent the use of `eval`. If that ever occurs, we should revert this
// to a normal `__non_webpack_require__` statement and simply document
// the Webpack warnings instead (telling users to ignore them).
//
// eslint-disable-next-line no-eval
const worker = eval('require')(getWorkerSrc());
return worker.WorkerMessageHandler;
}
await loadScript(getWorkerSrc());
Expand Down

3 comments on commit d370037

@shehan-mark
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When can we see this in a new build? Because we are just getting the issue like i explained here. mikecousins/react-pdf-js#186

@timvandermeij
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tracked in #11495.

@FredrikWallstrom
Copy link

@FredrikWallstrom FredrikWallstrom commented on d370037 Jun 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using rollup as bundler in my project and it throws a warning that eval is strongly discouraged. This holds me back from upgrading to the latest version of pdf.js, I don't want to suppress the eval warning over my whole project.

It has been a while since this was implemented, and I saw that the issue that is referenced in the code is closed (without solution/work around as I understand). Is there a work around/something else that can be done here to avoid the use of eval?

Please sign in to comment.