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.

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

Consider moving Webpack support to a separate package #9580

Closed
wojtekmaj opened this issue Mar 18, 2018 · 5 comments · Fixed by #11474
Closed

Consider moving Webpack support to a separate package #9580

wojtekmaj opened this issue Mar 18, 2018 · 5 comments · Fixed by #11474
Labels

Comments

@wojtekmaj
Copy link
Contributor

I propose moving webpack.js to a separate package.
Why?

If someone is instaling pdf.js, while NOT using Webpack, they will see the following warnings:

warning "yourpackage > pdfjs-dist@2.0.305" has unmet peer dependency "webpack@^2.0.0 || ^3.0.0".
warning "yourpackage > pdfjs-dist > worker-loader@1.1.1" has unmet peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".
warning "yourpackage > pdfjs-dist > worker-loader > schema-utils@0.4.5" has unmet peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

By doing so, we could get rid of worker-loader and webpack dependencies, making required download/build time/test time significantly smaller.

@wojtekmaj wojtekmaj changed the title Consider moving webpack support to a separate package Consider moving ebpack support to a separate package Mar 18, 2018
@wojtekmaj wojtekmaj changed the title Consider moving ebpack support to a separate package Consider moving Webpack support to a separate package Mar 18, 2018
@nfantone
Copy link

nfantone commented Jun 6, 2018

I believe the correct solution is outlined here: #9733. webpack should not be a "peerDependency" at all.

wojtekmaj referenced this issue in mikecousins/react-pdf-js Jun 26, 2018
@almothafar
Copy link

almothafar commented May 22, 2019

@nfantone yes, I don't see any reason for webpack to be in "peerDependency", these are "devDependencies" only has nothing to do with module after it is distributed.

It is been 1 year+ not sure if there is any fix for it, it is not major, but it is not really something need big efforts to solve either, just removing webpack from the "peerDependency" is enough.

https://github.com/mozilla/pdfjs-dist/blob/master/package.json#L19

@timvandermeij
Copy link
Contributor

Given the discussion on IRC at https://mozilla.logbot.info/pdfjs/20180606#c14862530-c14862634 it looks like splitting off Webpack into a separate package is a better option, or perhaps even removing the Webpack bundling altogether. I'm personally not too familiar with Webpack bundling, but that's what I infer from the discussion.

@yurydelendik Do you have an opinion on this? The current situation is not ideal because of the peer dependency and removing that is also not ideal given that warnings will pop up (see #9248). Is there a particular reason to keep the Webpack example in there?

@almothafar
Copy link

Do you really want worker-loader in dependencies?, maybe it should be in devDependencies, that's why you get another warning if you remove webpack from peers:
https://github.com/webpack-contrib/worker-loader/blob/master/package.json#L42

I see it is used here: https://github.com/mozilla/pdfjs-dist/blob/master/webpack.js#L18

This is the first time I see webpack published in distribution, for something should be working in the client side, as webpack as I know working for building the distribution not part of it.

@timvandermeij
Copy link
Contributor

Closing since the pull request above removed this dependencies altogether now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants