Skip to content

Commit

Permalink
[api-minor] Remove the Webpack-only npm dependencies from `pdfjs-dist…
Browse files Browse the repository at this point in the history
…` (PR 11418 follow-up)

Currently *all* users of `pdfjs-dist` are forced to install the `webpack` and `worker-loader` packages, despite the fact that they are *only* relevant if the `webpack.js` file is being used (with a custom Webpack build).
This really doesn't seem great, especially since those packages are the only remaining dependencies in the `pdfjs-dist` library, and it thus seem more reasonable overall that Webpack users handle those dependencies themselves.

To prevent unnecessarily cryptic runtime failures, when people update to newer `pdfjs-dist` versions, the `webpack.js` file was updated to explicitly check for the existence of the `worker-loader` package and error otherwise.
Furthermore, note that `webpack` was only listed as a dependency because of the `worker-loader` package itself (see issue 9248).

Obviously these changes may not be seen as great by Webpack users who rely on `pdfjs-dist`, since it forces them to handle the dependencies themselves, however it should improve things considerably for "general" users of `pdfjs-dist` by not burdening them with unnecessary dependencies.
These sort of changes are also in line with other recent changes, see PR 11418, which removed built-in fake worker loader code for specific JS builders/bundlers/frameworks. This work was prompted not only by a desire to simplify/clean-up old code, but also to lessen future support burden since the PDF.js contributors cannot be assumed to be experts in various JS bundlers.
  • Loading branch information
Snuffleupagus committed Jan 5, 2020
1 parent b833f84 commit a0cf67d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
9 changes: 9 additions & 0 deletions external/dist/webpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,17 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

"use strict";

try {
require.resolve("worker-loader");
} catch (ex) {
throw new Error(
"Cannot find the `worker-loader` package, please make sure that it's correctly installed."
);
}

var pdfjs = require("./build/pdf.js");
var PdfjsWorker = require("worker-loader!./build/pdf.worker.js");

Expand Down
6 changes: 0 additions & 6 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1548,12 +1548,6 @@ gulp.task(
homepage: DIST_HOMEPAGE,
bugs: DIST_BUGS_URL,
license: DIST_LICENSE,
dependencies: {
"worker-loader": "^2.0.0", // used in external/dist/webpack.json
},
peerDependencies: {
webpack: "^3.0.0 || ^4.0.0-alpha.0 || ^4.0.0", // from 'worker-loader'
},
browser: {
fs: false,
http: false,
Expand Down

0 comments on commit a0cf67d

Please sign in to comment.