mikecousins/react-pdf-js

Error in node_modules

Closed this issue · 14 comments

`ERROR in ./node_modules/react-pdf-js/lib/Pdf.js
02/02/2018 15:00:49Module not found: Error: Can’t resolve ‘pdfjs-dist/build/pdf.combined’ in ‘/app/node_modules/react-pdf-js/lib’
02/02/2018 15:00:49 @ ./node_modules/react-pdf-js/lib/Pdf.js 49:0-40
02/02/2018 15:00:49 @ ./node_modules/react-pdf-js/lib/index.js
02/02/2018 15:00:49 @ ./src/components/serviceHistory/index.js
02/02/2018 15:00:49 @ ./src/containers/adverts/index.js
02/02/2018 15:00:49 @ ./src/containers/main/index.js
02/02/2018 15:00:49 @ ./src/index.js
02/02/2018 15:00:49 @ multi ./src/index.js

I had this problem too, building v3.0.6 with the previously used version of pdfjs-dist works (2.0.203). I'm guessing there is some problem with that.

Ok no - it's a peer dependencies.
Make sure you have the correct pdfjs-dist version in your package.json:

"pdfjs-dist": "2.0.303", "react-pdf-js": "3.0.6",

@Driptap issue still persist
package json - web - visual studio code
sell my car - tootle - google chrome

Try manually deleting the react-pdf-js & pdfjs-dist folders from node modules, then installing the correct versions from the command line:

npm i -S react-pdf-js@3.0.6
npm i -S pdfjs-dist@2.0.303

I left the caret in by mistake on the previous comment, it still grabs latest version of pdfjs-dist

I'd remove the pdfjs-dist from your package.config and let this package handle it.

@Driptap Thanks.

What is the solution for the problem now? When letting react-pdf-js handle it, thats exactly when i'm getting the error and i can't ged rid of it.

Letting the package handle it seems to results in a newer version of pdfjs-dist (2.0.328) to be installed - this seems to be causing the Module Not Found error.

@sebastianmenge For now you should be able to install the correct version by adding "pdfjs-dist": "2.0.303" to your package.json, removing pdfjs-dist from node_modules and reinstalling.

@mikecousins maybe it is worth specifying the exact version of pdfjs-dist in react-pdf-js's package.json?

thanks @Driptap for clearing it up. i was just confused by the answer and thought the way to go would be not putting the pdfjs-dist in the package. working now.

@mikecousins When there will be release with corrected bugs?

Currently, in my project I had to add

    "pdfjs-dist": "2.0.139",
    "react-pdf-js": "3.0.0",

to this specific version of pdfjs-dist. It appears that the newer version of pdfjs-dist is missing pdf.combined JS file, which even the master branch of this repo depends on still.

Which, is probably a good thing, because that file (old version) injects an override of Object.defineProperty which does not work as expected: https://raw.githubusercontent.com/mozilla/pdfjs-dist/846bbae9bcb9ce06ca3d04da9e99beecb9900e82/build/pdf.combined.js

Object.defineProperty = function objectDefineProperty(obj, name, def) {
      delete obj[name];
      if ('get' in def) {
        obj.__defineGetter__(name, def['get']);
      }
      if ('set' in def) {
        obj.__defineSetter__(name, def['set']);
      }
      if ('value' in def) {
        obj.__defineSetter__(name, function objectDefinePropertySetter(value) {
          this.__defineGetter__(name, function objectDefinePropertyGetter() {
            return value;
          });
          return value;
        });
        obj[name] = def.value;
      }
    };

This library should use the non "combined" version of this file, no?

@Driptap I can confirm that the Object.definePropert tomfoolery is removed in this version of pdfjs-dist (https://raw.githubusercontent.com/mozilla/pdfjs-dist/d90724150fdff57ba76c25e2f3167d12105ba2a1/build/pdf.combined.js, find "defineProperty =" missing). However, if I have dependencies

  "dependencies": {
    "pdfjs-dist": "2.0.303",
    "react-pdf-js": "3.0.6"
  },

I still end up with the new version of pdfjs-dist, which is incompatible, in the local node_modules/ of react-pdf-js


➜  myproj git:(master) ✗ npm list pdfjs-dist
myproj@0.6.26 /Users/rosenbek/myproj
├── pdfjs-dist@2.0.303 
└─┬ react-pdf-js@3.0.6
  └── pdfjs-dist@2.0.332 

and version 2.0.332 is missing pdf.combined.js.

The PR above should fix this problem because it is in the dependencies of react-pdf-js that this issue arises (https://github.com/mikecousins/react-pdf-js/pull/62/files), however, it still stands that isn't it a bad idea to use the pdf.combined.js file? It seems like so much bloat is in there? And like I said, that polyfill/hijacking of Object.defineProperty seems to behave differently from native Object.defineProperty.

I have merged and released the change to hardcode the version to the old one.

I like this talk about switching away from pdf.combined but I'm unsure of the ramifications of it.

@mikecousins We should indeed get rid of the pdf.combined 🙂
Since the provided polyfills won't be included anymore, that would be a major version bump.