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

core-js placed in dev-dependencies instead of dependencies #8890

Closed
wojtekmaj opened this issue Sep 10, 2017 · 4 comments
Closed

core-js placed in dev-dependencies instead of dependencies #8890

wojtekmaj opened this issue Sep 10, 2017 · 4 comments

Comments

@wojtekmaj
Copy link
Contributor

In shared/compatibility.js, we have require() pointed at core-js/fn/typed/uint8-clamped-array.

I can see that in pdf.js's package.json core-js is defined, but in devDependencies - "core-js": "^2.5.0",.

When using this library somewhere, installing it via npm's pdfjs-dist, devDependencies are obviously not being installed. Thus, attempting to load shared/compatibility.js or any library that relies on shared/compatibility.js (i.e. shared/util.js) will fail.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Sep 10, 2017

When using this library somewhere, installing it via npm's pdfjs-dist, devDependencies are obviously not being installed. Thus, attempting to load shared/compatibility.js or any library that relies on shared/compatibility.js (i.e. shared/util.js) will fail.

Note that compatibility.js, since PR #8102, is now bundled directly into both pdf.js and pdf.worker.js in generic builds (such as the ones available in pdfjs-dist), which should alleviate the need to include it manually.
Furthermore, pdfjs-dist still offers a pre-built version of the web/compatibility.js file, see https://github.com/mozilla/pdfjs-dist/blob/master/web/compatibility.js, and using that would also avoid these issues.

So, in closing, it's not really clear to me that any changes need to be made here. (Especially since the files found in https://github.com/mozilla/pdf.js/tree/master/src aren't really intended to be used as-is, without building them first.)

@wojtekmaj
Copy link
Contributor Author

Thanks for your reply. So does it mean using /display/svg.js is not recommended and there is another way of making SVG rendering using built version of PDF.js files? I tried to follow sample file as close as possible.

Not sure if that changes anything, but I actually don't go to pdf.js/src, I'm importing the file from pdfjs-dist/lib.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Sep 10, 2017

Thanks for your reply. So does it mean using /display/svg.js is not recommended and there is another way of making SVG rendering using built version of PDF.js files? I tried to follow sample file as close as possible.

SVGGraphics is already bundled into https://github.com/mozilla/pdfjs-dist/blob/master/build/pdf.js, so you should be able to access it from there when used in a production environment.

Not sure if that changes anything, but I actually don't go to pdf.js/src, I'm importing the file from pdfjs-dist/lib.

The https://github.com/mozilla/pdfjs-dist/tree/master/lib folder mostly exists in order to support unit-testing with Travis (using Node.js).

@wojtekmaj
Copy link
Contributor Author

Oh, I wasn't aware it's already there. Works like a charm.
Thank you!

PS. Amazing work! Super excited for what will come :)

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

2 participants