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

Not working using browserify, but I don't know if it's still the cause #33

Closed
lorenzos opened this issue Jul 18, 2017 · 5 comments
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@lorenzos
Copy link

lorenzos commented Jul 18, 2017

I tried 1.8.3 with Browserify, and I still have problems in running my app. Instead of trying to build the provided sample as I did discussing nnarhinen/react-pdf#39, this time I tried writing a minimal app from scratch. The issue looks different from before.

git clone https://bitbucket.org/lorenzostanco/reactpdf-browserify-test
cd reactpdf-browserify-test/
npm install
npm run build
npm run serve

PDF loading fails, and in the console I see:

GET http://127.0.0.1:8080/build.worker.js 404 (Not Found)
Warning: Setting up fake worker.
GET http://127.0.0.1:8080/build.worker.js 404 (Not Found)
@wojtekmaj
Copy link
Owner

Hey,
it looks like I don't have access to this repository.
Judging by the error, though, here's what I know:
On Webpack, we use worker-loader that passes it to pdf.js automatically.
On Browserify, you have to copy the loader to the specific place on your own. This is because when pdf.js doesn't get one in params, it will attempt to get it from default URL.
React-PDF should work okay without this file. But yes, it is highly recommended that you get it in place for performance reasons.

Consider making something like this using gulp like they do in their official example?
https://github.com/mozilla/pdf.js/blob/master/examples/browserify/gulpfile.js

@lorenzos
Copy link
Author

lorenzos commented Jul 19, 2017

Sorry, I gave you the repo URL using SSH, that needs my SSH key. Use:

git clone https://bitbucket.org/lorenzostanco/reactpdf-browserify-test/src

On Browserify, you have to copy the loader to the specific place on your own

Yes, after copying pdfjs-dist/build/pdf.worker.min.js to the path shown in the 404 error, it works! Thank you. I didn't tried the gulp task, but from a quick look it's clear how it works.

React-PDF should work okay without this file

It does not. Those errors are shown, and the PDF fails to load (it hangs on "Loading PDF..." forever).

it is highly recommended that you get it in place for performance reasons

Can I ask you if you think it's relevant, considering I'm just rendering a single page when my app loads? If not, I prefer having a smaller bundle. Giving I'll be able to make React-PDF work without it...

@lorenzos
Copy link
Author

lorenzos commented Jul 19, 2017

Replying to the comment you wrote on the wrong issue: 😄

One more question. Does using your version of the fix, with 'pdf.combined.js' does NOT result in this warning or only gets rid of 404's? If pdf.combined.js is working great maybe it's the path we should follow in 2.0.

Using nnarhinen/react-pdf#43 the 404 errors go away, but I still get the Warning: Setting up fake worker

@wojtekmaj
Copy link
Owner

wojtekmaj commented Jul 19, 2017

Hah, thank you for being so observant! :D

Answering your question about relevancy, it does not really work this way, that it loads several hundred kb and the only advantage is a separate thread. PDF.js still needs everything that is inside pdf.worker.js, whether or not it is treated as a worker. Disabling worker just causes it to load in one thread, which gives you no benefits whatsoever. Long story short - worker has to be there.

I just released v2.0.0-beta and I've made sure to make instructions clear about this. Let me know if I could help you with anything else!

@wojtekmaj wojtekmaj added this to the 2.0.0 milestone Jul 19, 2017
@wojtekmaj wojtekmaj self-assigned this Jul 19, 2017
@wojtekmaj wojtekmaj added the bug Something isn't working label Jul 19, 2017
@wojtekmaj
Copy link
Owner

By the way, to upgrade your test project to 2.0.0:

  1. Update react-pdf:
    "react-pdf": "^2.0.0-beta"
  1. Add commands to package.json:
    "build": "npm run copy-worker && npm run build-browserify",
    "build-browserify": "browserify index.jsx -t [ babelify --presets [ es2015 react stage-2 ] ] --outfile build.js",
    "copy-worker": "copy node_modules\\pdfjs-dist\\build\\pdf.worker.js pdf.worker.js",
  1. Modify index.jsx:
import React, { Component } from 'react';
import { render } from 'react-dom';
import { Document, Page } from 'react-pdf';

render(<Document file="sample.pdf"><Page pageIndex={2} /></Document>, document.getElementById('app'));

Enjoy :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants