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

peer dependency for pdfjs-dist #461

Closed
maxisam opened this issue Apr 3, 2019 · 6 comments
Closed

peer dependency for pdfjs-dist #461

maxisam opened this issue Apr 3, 2019 · 6 comments
Milestone

Comments

@maxisam
Copy link

maxisam commented Apr 3, 2019

"peerDependencies": {

Since pdfjs-dist is in the dependency already. Do we still need it in peerDependency?

@dhyanaswain
Copy link

it's better to add the PDF.js package to your NodeModule.
As every pdf viewer use pdf.js in the backend to load any pdf files.

@maxisam
Copy link
Author

maxisam commented Apr 4, 2019

Well, but I don't need PDF.js, it kinda pollutes my package.json if every package does this.

@dhyanaswain
Copy link

Yes, some of the packages generates this "peerDependencies"

@JoostK
Copy link

JoostK commented Aug 15, 2019

I would like to raise this issue as well. We were seeing an issue in https://github.com/mgechev/ngcc-validation/pull/220 in the situation where there was an additional package with a peer dependency on pdfjs-dist, so pdfjs-dist was present as dependency of the project itself, to satisfy said dependency. This caused an issue with duplicate installations of pdfjs-dist, probably because of the direct dependency on pdfjs-dist from ng2-pdf-viewer.

Note that it is strange to list a package both as dependency and as peer dependency, as the former will cause the requested version to be installed, whereas the latter is just a signal to library consumers to let people know they need to include the peer dependency as a dependency of their own.

For libraries, it is typically considered best practice to mark dependencies as peer dependency, as it avoids ending up with multiple versions of the some package "A" if there's other packages that depend on "A". Note, however, that removing the direct dependency would be a breaking change, as library consumers would suddenly be responsible for listing the dependency in their own package.json.

@maxisam
Copy link
Author

maxisam commented Apr 4, 2020

this causes an issue in Angular9 now

Because ngcc can't figure out which package it should use. We should just keep one.

ERROR in ./node_modules/ng2-pdf-viewer/__ivy_ngcc__/node_modules/pdfjs-dist/build/pdf.js
Module not found: Error: Can't resolve 'fs' in 'C:\Users\xxx\WorkProject-git\xxxx\xxxx\ClientApp\node_modules\ng2-pdf-viewer\__ivy_ngcc__\node_modules\pdfjs-dist\build'

To fix above issue, remove pdfjs-dist in your package.json

VadimDez added a commit that referenced this issue Apr 4, 2020
@VadimDez VadimDez added this to the 6.2.0 milestone Apr 14, 2020
@VadimDez
Copy link
Owner

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

No branches or pull requests

4 participants