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 when using Browserify, fires a request to [object Object] #39

Closed
lorenzos opened this issue Mar 15, 2017 · 2 comments
Closed

Comments

@lorenzos
Copy link

lorenzos commented Mar 15, 2017

I wanted to use your component in my app, but I didn't manage to make it work (details below). After trying your sample directly from your repo, I noticed the issue comes from building my app using Browserify instead of Webpack. I can't get why Browserify fails, so I'm going to illustrate all my findings.

Sample built with Webpack

$ git clone https://github.com/nnarhinen/react-pdf.git
$ cd react-pdf
$ npm install
$ npm run build-sample

sample/index.html works great.

Sample built with Browserify

Need to edit sample.jsx, adding JSX extension in import ReactPDF from '../src/react-pdf.jsx' and removing import './sample.less'.

$ npm install babelify 
$ npm install babel-preset-es2015 babel-preset-react babel-preset-stage-2
$ npm install babel-plugin-transform-class-properties babel-plugin-transform-es2015-modules-commonjs
$ cd src-sample
$ browserify sample.jsx -t [ babelify --presets [ es2015 react stage-2 ] --plugins [ transform-class-properties transform-es2015-modules-commonjs ] ] --outfile ../sample/sample.js

sample/index.html now does not work, in console I can see that a request is made to http://.../[object%20Object], which obviously fails. Searching your repo with this information, I found commit a3ab1c3: it says it fix my issue, but it does not. I tried to manually revert the commit, editing src/react-pdf.jsx, removing that line and:

$ npm run build-source
$ browserify sample.jsx -t [ babelify --presets [ es2015 react stage-2 ] --plugins [ transform-class-properties transform-es2015-modules-commonjs ] ] --outfile ../sample/sample.js

Same error in console, but followed by three more lines:

    GET http://.../[object%20Object] [HTTP/1.1 404 Not Found 0ms]
 -> Warning: Setting up fake worker.
 -> GET http://.../[object%20Object] [HTTP/1.1 404 Not Found 0ms]
 -> NetworkError: Failed to load worker script at "[object Object]"

Later I'll find that, in fact, the issue is somehow related to the worker.

"Older" version of your component works instead!

After many attempt to trace where the problem is, I gave up and tried react-pdfjs instead. This is another React component that uses PDF.js, so I was going to find out if the Browserify issue was with PDF.js itself, or not.

To my surprise, this component worked well. Its README says that it is nothing but a port of your react-pdf! Comparing sources I concluded react-pdfjs is in fact based on a much older version of react-pdf. So I went on in finding where the Browserify issue may arise.

First thing I noticed is that the "old" code in react-pdfjs is not only missing the additional line from a3ab1c3, but it also requires 'pdfjs-dist/build/pdf.combined' instead of 'pdfjs-dist/build/pdf' plus 'pdfjs-dist/build/pdf.worker.js'. So I tried editing your src/react-pdf.jsx again:

import React, { Component, PropTypes } from 'react';

require('pdfjs-dist/web/compatibility');
require('pdfjs-dist/build/pdf.combined');

export default class ReactPDF extends Component {
	// ...
};

Finally:

$ npm run build-source
$ browserify sample.jsx -t [ babelify --presets [ es2015 react stage-2 ] --plugins [ transform-class-properties transform-es2015-modules-commonjs ] ] --outfile ../sample/sample.js

And now your sample built with Browserify works! I tried rebuilding it again with Webpack, and it works too.

Questions

Unfortunately I didn't find through commits why your code is using pdfjs-dist/build/pdf instead of pdf.combined like the older port does, nor I found anything in existing issues related to the aforementioned commit a3ab1c3. So I'm asking you now, before proposing a change to your code.

The only thing I found interesting is this issue on the port, which mentions pdf.combined and says it should not be used because it's like disabling worker, which anyway is what you already did in a3ab1c3.

My goal is to make your component work well in my app, which I have to build with Browserify, so I'd be happy if we can find any different and more "explicit" solution. I'm not willing to use the old port in my app because it's missing features like width prop and onDocumentX/onPageX callbacks with detailed arguments.

Hope I'd been helpful some way.

@wojtekmaj
Copy link
Contributor

Hi!

Thank you for your post and sorry for no reply.

I dug into file history and I have not found any point in which we were using pdfjs-dist/build/pdf.combined instead of 'pdfjs-dist/build/pdf.
Disabling worker did nothing for me performance-wise, despite testing on pretty heavy PDFs on multiple browsers, so I don't think using .combined file would make it any worse.
If that's going to be the way to fix [Object object] thing, then it might be very much worth trying!

lorenzos added a commit to lorenzos/react-pdf that referenced this issue May 11, 2017
maxwells added a commit to maxwells/react-pdf that referenced this issue Jun 17, 2017
@wojtekmaj
Copy link
Contributor

Hi @lorenzos, version 1.7.0 has just been released and it should fix your issue :) Let me know if it didn't.

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