wojtekmaj / react-pdf

Display PDFs in your React app as easily as if they were images.
https://projects.wojtekmaj.pl/react-pdf
MIT License
9.24k stars 877 forks source link

add workers in exports #1584

Closed francisco-sanchez-molina closed 1 year ago

francisco-sanchez-molina commented 1 year ago

This pull-request addresses an issue with the import of the PDF worker in the "react-pdf" package. Due to recent changes in the package, the previous import path for the PDF worker no longer works. This PR proposes a solution to fix the import of the PDF worker and ensure it works as intended.

Proposed Changes

Updated "package.json": In the "package.json" file of the "react-pdf" package, two new export paths have been added to allow access to the PDF worker module:

"exports": {
     ....
    "./worker/legacy/pdf.worker.entry.js": "./node_modules/pdfjs-dist/legacy/build/pdf.worker.entry.js",
    "./worker/pdf.worker.entry.js": "./node_modules/pdfjs-dist/build/pdf.worker.entry.js"
},

With this change we can do:

export const getWorker = () => {
  try {
    return require('react-pdf/worker/legacy/pdf.worker.entry.js');
  } catch (e: unknown) {
    return require('pdfjs-dist/legacy/build/pdf.worker.entry.js');
  }
};

pdfjs.GlobalWorkerOptions.workerSrc = getWorker()
wojtekmaj commented 1 year ago

Thanks for this.

Can you please tell me how are you using pdf.worker.entry file? It's not documented anywhere and is used only internally at the moment.

francisco-sanchez-molina commented 1 year ago

Thanks for this.

Can you please tell me how are you using pdf.worker.entry file? It's not documented anywhere and is used only internally at the moment.

Hello! Thank you for responding.

We are using the project within an NX monorepo with React and Webpack. Until now, we were loading it like this:

import React, { useDeferredValue } from 'react';
import { pdfjs } from 'react-pdf';
import { useMeasure } from 'react-use';
import { Box, SxProps } from '@mui/material';

export const getWorker = () => {
  try {
    return require('react-pdf/node_modules/pdfjs-dist/legacy/build/pdf.worker.entry.js');
  } catch (e: unknown) {
    return require('pdfjs-dist/legacy/build/pdf.worker.entry.js');
  }
};

pdfjs.GlobalWorkerOptions.workerSrc = getWorker();

However, it has stopped working with the latest version. If I use pdfjs-dist/legacy/build/pdf.worker.entry.js, it works, but I have to keep the version aligned in the package.json.

wojtekmaj commented 1 year ago

I'm confused how your changes would be able to fix that. Note that the exports defined are relative to react-pdf package.json, and there's no such thing as ./worker directory there.

I would also advise you to configure workerSrc using one of the recommended options to avoid uncommon issues.

francisco-sanchez-molina commented 1 year ago

Ok! I'll try another approach to configure it if you don't think this is highly recommended. Previously, without the exports configuration, I could use require to access the specific worker version that react-pdf had in his node-modules. Now, with the exports configured, it's showing this error:

WARNING in ../../../libs/bss/documents/signature/signature-step-document-read-accept/src/lib/components/PdfViewer.tsx 13:11-65
Module not found: Error: Package path ./worker/legacy/pdf.worker.entry.js is not exported from package /Users/psm1984/monorepo/front/node_modules/react-pdf (see exports field in /Users/psm1984/monorepo/front/node_modules/react-pdf/package.json)

webpack compiled with 1 warnings (fd589530e59b86de)

For me, this approach is simpler because it leverages the worker from the appropriate version that react-pdf has, without having to install a pdfjs-dist version that might be different and not work. However, I'm not an expert in npm packages, and I'm unsure if adding what I've mentioned to the export is problematic.

thank you very much for the library and for the support!

wojtekmaj commented 1 year ago

It actually appears to be an error inside the error message! Look closely, makes no sense at all :D

Regarding pdfjs-dist: You should not need to install it manually at all. If you don't (and neither any of the other dependencies you might have), you'll have one copy, sitting directly in your node_modules. It's only after you add a second, "more important" version of pdfjs-dist, then "my" pdfjs-dist is forced to move to node_modules/react-pdf/node_modules.

francisco-sanchez-molina commented 1 year ago

Thank you very much for the support! The issue I had appeared with the latest version, but there was a TypeScript type package that was conflicting with the version of pdfjs-dist.