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

Please help me testing React-PDF 5.0 beta update! #495

Closed
wojtekmaj opened this issue Dec 7, 2019 · 41 comments
Closed

Please help me testing React-PDF 5.0 beta update! #495

wojtekmaj opened this issue Dec 7, 2019 · 41 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@wojtekmaj
Copy link
Owner

wojtekmaj commented Dec 7, 2019

Hi,
React-PDF v5.0.0-beta.5 has been released introducing a few major updates:

  • React-PDF now ships with ES6 Modules
  • Updated pdf.js from 2.1.266 to 2.4.456.

Breaking changes:

  • You will need to change your imports to bundler-specific entry files, for example instead of:
    import { Document } from 'react-pdf/dist/entry.webpack';
    
    you'll now write:
    import { Document } from 'react-pdf/dist/esm/entry.webpack';
    
  • Dropped support for IE 11 due to pdf.js dropping support

so upgrade should be a piece of cake.

Please share your experiences - whether it works correctly in your project, doesn't break the build, and so on.

Thanks!

@wojtekmaj wojtekmaj added the help wanted Extra attention is needed label Dec 7, 2019
@wojtekmaj wojtekmaj self-assigned this Dec 7, 2019
@wojtekmaj wojtekmaj pinned this issue Dec 7, 2019
@Edll
Copy link

Edll commented Dec 9, 2019

Hello,

i have updated from version 4.1.0 to this one and changed the imports. I could not detect any problems while building or executing.

@robsco-git
Copy link
Contributor

robsco-git commented Dec 17, 2019

Hi

I just tested the new version: "react-pdf": "^5.0.0-beta"

In a testing (production build) environment, the following is logged to the console of Chromium:
Refused to execute script from 'https://<redacted>/13.67eff32e9ffe66eca602.chunk.worker.js' because its MIME type ('text/html') is not executable, and strict MIME type checking is enabled.
And:
Warning: Setting up fake worker.

The 13.67eff32e9ffe66eca602.chunk.worker.js file does not exist. It was never generated by Webpack. Webpack did generate two worker .js files that include "pdf" strings:
vendors~pdfjsWorker.67eff32e9ffe66eca602.chunk.js
17fac81207a6368c84f4.worker.js

This is a SPA so the MIME type error is due to the frontend 404 page being returned from an invalid route.

I imagine this is probably a Webpack configuration issue.
"webpack": "^4.41.3"
"webpack-cli": "^3.3.10"

I am importing the Document and Page components each in a separate file and component:
import { Document } from 'react-pdf/dist/esm/entry.webpack';
import { Page } from "react-pdf/dist/esm/entry.webpack";

It doesn't happen with Webpack config:
mode: "development"
But does happen with:
mode: "production"

My Webpack config is a bit long/complicated so I'll see if I can isolate this to a particular set of config options.

@robsco-git
Copy link
Contributor

robsco-git commented Dec 17, 2019

My Webpack config is a bit long/complicated so I'll see if I can isolate this to a particular set of config options.

I boiled this down to the Webpack config option: optimization.sideEffects.

With mode: "production" I needed to set:

optimization: {
    sideEffects: false,
}

This got the production build to work without falling back to the "fake worker" at runtime.

I imagine this is related to:

"sideEffects": false,

@robsco-git
Copy link
Contributor

Removing the line

"sideEffects": false,

Worked for me without having to include:

optimization: {
    sideEffects: false,
}

in my Webpack config with mode: "production".

Sorry for sort of hijacking this thread. I'll create a pull request shortly.

@brentmitch
Copy link

I've tested 5.0.0-beta.2 both in development mode and a build that I ran locally. I didn't see any errors on the screen or in the console. I tested a wide variety of documents. It'd be nice to get the Worker Blob fix #452 in.

@vincentdegheyndt
Copy link

vincentdegheyndt commented Mar 4, 2020

I have tried running it with typescript but to no avail. Is there an up to date version of the @types/react-pdf that's compatible with the beta ?

I was hoping the beta might fix the #280 dependancy issue. I have used the cdn workaround and got react-pdf to work but would like to get rid of the error message.

@wojtekmaj
Copy link
Owner Author

There are no substantial differences between 4.x and 5.x API, so types for 4.x should work 🤔

@vincentdegheyndt
Copy link

Weird. Maybe I did something wrong.

import { Document } from 'react-pdf/dist/esm/entry.webpack';
import { Page } from 'react-pdf/dist/esm/entry.webpack';
import { pdfjs } from 'react-pdf/dist/esm/entry.webpack';

I used these lines to import the files and I get "Could not find a declaration file for module 'react-pdf/dist/esm/entry.webpack'."

@burtonator
Copy link

I'm considering using this in Polar (https://getpolarized.io/) as I need to rework our PDF viewer to be a bit more modern, support mobile + tablets, etc.

For us this would be a major component and I need it to be really solid.

There are a few things that are missing with react-pdf ... which you seem to have done a great job with btw!

  • PDFFindController isn't currently used? It's definitely a bit tough to figure out but I think I figured how their API (which is a big ugly).

  • Typescript. Would be nice if this was done completely in Typescript. How would you feel about a PR that migrated everything to TS?

  • The latest PDF.js (which just dropped 5x days ago) supports setting the background color of the canvas. I want to play with a dark mode and sepia mode for our users (top requested feature) so working on a port to the latest pdf.js might be done eventually as getting this done is a big priority for us.

I figure if the Typescript change is too much for you I can just work on the PDFFindController and latest PDF.js migration and then maybe you could cherrypick those and we would just maintain a fork for a bit.

Thoughts?

@wojtekmaj
Copy link
Owner Author

@vincentdegheyndt That would be @DefinitelyTyped typings issue.

@wojtekmaj
Copy link
Owner Author

@burtonator

PDFFindController isn't currently used? It's definitely a bit tough to figure out but I think I figured how their API (which is a big ugly).

I experimented with that in the past a little, but didn't find a good way to implement it as understandable React-based API. It's a pain - I'd love to see some concepts around this.

Typescript. Would be nice if this was done completely in Typescript. How would you feel about a PR that migrated everything to TS?

Since majority of our contributors, including the main one being me, are not very familiar with TypeScript, I see this as a thing that could potentially slow us down by a large chunk. We would need someone with this knowledge available as a fully fledged team member.

The latest PDF.js (which just dropped 5x days ago) supports setting the background color of the canvas. I want to play with a dark mode and sepia mode for our users (top requested feature) so working on a port to the latest pdf.js might be done eventually as getting this done is a big priority for us.

Mozilla doesn't have very good history with keeping their version numbers in line with semver guidelines, and that's why it's set to exact match in our dependencies. Still, more often than not, you can go ahead and try newer PDF.js version on your own (using yarn resolutions for example) without any major issues.

@wojtekmaj
Copy link
Owner Author

wojtekmaj commented Apr 27, 2020

FYI: next beta, v5.0.0-beta.3 is out :)

@aleyan
Copy link

aleyan commented Apr 28, 2020

Updated to v5.0.0-beta.3 from 4.1.0. I did not change how we import and stuck with current:

import { pdfjs, Document } from 'react-pdf';
pdfjs.GlobalWorkerOptions.workerSrc = `//cdnjs.cloudflare.com/ajax/libs/pdf.js/${pdfjs.version}/pdf.worker.js`;

Importing like import { Document } from 'react-pdf/dist/esm/entry.webpack'; did not seem to work in our webpack project.

No other issues. The newer version of PDF.js fixed a rendering issue in some of our pdfs, so this update has been very welcome.

Thank you!

@xeger
Copy link

xeger commented Apr 29, 2020

Updated to 5.0.0-beta.3 and my app worked well.

@pavanbi2i
Copy link

Updated to 5.0.0-beta.3

Uncaught TypeError: Cannot read property 'destroy' of undefined
at Document.componentWillUnmount (Document.js:457)

457: this.loadingTask.destroy();

may be we need to null check loadingTask

@wojtekmaj
Copy link
Owner Author

React-PDF 5.0.0-beta.4 is out. Hopefully a final one (although Webpack's 18th beta is making me feel a little better 😅)

pdf.js 2.4.456 is under the hood, although an ES5 build as I saw multiple compatibility issues with the new build, and I didn't want to introduce additional breaking changes between beta releases.
Fix for @pavanbi2i's issue is also included.

@samalexander
Copy link

With v5.0.0-beta.4 I am seeing the following error in the console. Anyone else experiencing the same?

image

@danieltott
Copy link
Contributor

Same - posting the error text for searchability:

getGlobalEventBus is deprecated, use a manually created EventBus instance instead.

A couple notes:

@danieltott
Copy link
Contributor

I tried the following using yarn resolutions to try to use pdfjs-dist 2.5.207, but pdfjs was undefined 🤷‍♂️:

"resolutions": {
  "react-pdf/pdfjs-dist": "2.5.207"
},

@MaximilienMeyer
Copy link

Hi @wojtekmaj,

Is npm updated too ? I've run npm update react-pdf and nothing happen.
Should I delete this package and get the beta from github ?

@danieltott
Copy link
Contributor

Looks like the EventBus deprecation was noted in the 2.4.456 release, and was dealt with in this closed PR but was not handled in the PR that finally updated react-pdf to pdfjs-dist 2.4.456

@danieltott
Copy link
Contributor

Here's PR with a fix! #590

@wojtekmaj
Copy link
Owner Author

Hi @wojtekmaj,

Is npm updated too ? I've run npm update react-pdf and nothing happen.
Should I delete this package and get the beta from github ?

Yes it is, but beta != latest. You will need to manually opt in to beta. npm update won't work for versions not marked as stable.

@MaximilienMeyer
Copy link

Hi @wojtekmaj,
Is npm updated too ? I've run npm update react-pdf and nothing happen.
Should I delete this package and get the beta from github ?

Yes it is, but beta != latest. You will need to manually opt in to beta. npm update won't work for versions not marked as stable.

You rock, thanks for fast answer.

@oceandrama
Copy link

oceandrama commented Jun 15, 2020

@wojtekmaj At version beta.4 I've got the following error

Uncaught SyntaxError: Unexpected token '<'
Error: Setting up fake worker failed: "Cannot read property 'WorkerMessageHandler' of undefined"

I import components like this

import { Document, Page } from "react-pdf";

beta.3 works perfectly

@wojtekmaj
Copy link
Owner Author

That's odd - the way we load worker through Webpack loaders is unchanged and https://cdnjs.cloudflare.com/ajax/libs/pdf.js/2.4.456/pdf.worker.js is available, so if you opted for the defaults, whichever they were, everything should work just fine. Can you check your network tab what worker is returning "404" = index.html file served instead?

@SamuelMS
Copy link

@wojtekmaj I get massive performance degradation and some odd behavior when upgrading from 5.0.0-beta.2 to 5.0.0-beta.4. We use the "standard" instructions to serve the pdf.js worker locally.

@wojtekmaj
Copy link
Owner Author

@SamuelMS Any specific case? Would love to look into that if you have a minimal reproduction repo + PDF sample.

@SamuelMS
Copy link

@SamuelMS Any specific case? Would love to look into that if you have a minimal reproduction repo + PDF sample.

Ouch. Yeah, this is buried pretty deep into proprietary code – with infinite loading and coupled to a few other libraries. Still, the only piece that changed here is the react-pdf version bump.

Totally understand you need more to go on here... I can try getting an example together, but it'll take me some time.

@negarineh
Copy link

With v5.0.0-beta.4 I am seeing the following error in the console. Anyone else experiencing the same?

image

Any update on this one? I'm receiving same warning on beta.4
I tried all ways here, still same.

@wojtekmaj
Copy link
Owner Author

@SamuelMS Perhaps you could share Chrome DevTools measurements then? They could make it easier to spot where the slowdown happens.

Once recorded, you can use ⬇️ button to save the measurements.

obraz

@wojtekmaj
Copy link
Owner Author

@negarineh That should be resolved with #593, which will be out in the next release. Hopefully stable one... 🤐

@casulit
Copy link

casulit commented Jul 18, 2020

Same error with @negarineh when is the next release @wojtekmaj? :)

@pohlman
Copy link

pohlman commented Jul 21, 2020

Just an FYI that the latest breaks IE11 due to pdf.js deciding to remove support for it (annoyingly in a minor version).

The pdfjs-dist/es5 imports you switched to are fine, but things imported from pdfjs-dist/lib like PDFLinkService aren't transpiled, and still have classes in the final output. It took a while to figure out what was going on, and we solved it by configuring babel to that transpile imports from that module.

The CDN link for pdf.worker.js is also no longer transpiled, so we switched to
pdfjs.GlobalWorkerOptions.workerSrc = `//unpkg.com/pdfjs-dist@${pdfjs.version}/es5/build/pdf.worker.js`;

@wojtekmaj
Copy link
Owner Author

wojtekmaj commented Jul 27, 2020

Uh, oh. Unfortunately this means that we will probably need to drop IE11 support too for the final 5.0 release in order to do any progress and allow future updates.

The good news is that if that's the case, we will cherry-pick multiple fixes and improvements from 5.0 to 4.x branch.

@mdaffan
Copy link

mdaffan commented Jul 29, 2020

getGlobalEventBus is deprecated, use a manually created EventBus instance instead
Getting this error in 5.0.0-beta.4

@wojtekmaj
Copy link
Owner Author

wojtekmaj commented Aug 4, 2020

5.0.0-beta.5 has been released:

  • fix for deprecated globalEventBus
  • smaller bundle size
  • officially dropped support for IE 11 (should have picked up that sooner - sorry about that).

React-PDF home page already uses it 👀

Also, 4.2.0 has been released with long awaited fixes and improvements introduced during 5.0 development.

@balrampariyarath
Copy link

balrampariyarath commented Aug 5, 2020

Hi @wojtekmaj,

I already see that #415 has been addressed in the 5.0 beta but when will it be available in a stable release? Will there be a separate release between 4.2.0 and 5 addressing this issue?

@wojtekmaj
Copy link
Owner Author

@balrampariyarath Unfortunately not. #280/#415 has been addressed not in React-PDF per se, but in one of pdf.js updates, which also introduced a breaking change (dropping IE 11 support). As such, 4.x branch is stuck on this pdf.js version forever.

@wojtekmaj
Copy link
Owner Author

Well, almost a year in beta, that was long enough in testing. v5.0.0 is out. React-PDF home page already uses it.

Thanks everyone for sharing your comments and helping to push this release out.

@SamuelMS
Copy link

@SamuelMS Perhaps you could share Chrome DevTools measurements then? They could make it easier to spot where the slowdown happens.

Once recorded, you can use ⬇️ button to save the measurements.

obraz

@wojtekmaj Apologies for the delay here – finally got around to this. Opened a new issue (#663) to make it easier to track.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests