search

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.52k stars 894 forks source link

react-scripts: 2.0.3 breaks dependencies in lib/display/api.js and build/pdf.js v3.0.5 #280

Closed lilcorey closed 4 years ago

lilcorey commented 6 years ago

Updating create-react-app to v2.0.3 has resulted in this node module to not load PDFs. 

Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/lib/display/api.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/lib/display/api.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/lib/display/api.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/lib/display/api.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js

Describe solutions you've tried

Deleting the module and reinstalling via npm install has resulted in the same issue.

Additional information

If applicable, add screenshots (preferably with browser console open) and files you have an issue with to help explain your problem.

Environment

wojtekmaj commented 6 years ago

Hello, thanks. This should probably be reported to mozilla/pdf.js.

Webpack 4 is not supported officially at the moment, but will be in version 4 - have you tried using it instead by installing react-pdf@next?

chiragv508 commented 6 years ago

yes.it's incompatile with react-script-2

wojtekmaj commented 6 years ago

Check your console and look if your error is mentioned in Known issues. I just updated it while I was handling a similar issue.

wojtekmaj commented 6 years ago

Create React App is not yet compatible with worker-loader. It will be once facebook/create-react-app#3934 gets merged. For now, users of Create React App both 1.x and 2.x need to use generic solution.

tzieleniewski commented 6 years ago

@wojtekmaj May you confirm that generic solution means

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

and manual copy of the pdf.worker.js file from pdfjs-dist/build is project's output folder.

wojtekmaj commented 6 years ago

Yes, that is the generic solution @tzieleniewski. You can also use CopyWebpackPlugin to do it with every build.

Krivega commented 5 years ago

@tzieleniewski @wojtekmaj small automation: add "postinstall": "cp node_modules/pdfjs-dist/build/pdf.worker.min.js public/pdf.worker.min.js" to the scripts section in the package.json

mikemclin commented 5 years ago

So to confirm, there is a working solution, but we're still unable to get rid of the Critical dependency: require function is used in a way in which dependencies cannot be statically extracted warning.

wojtekmaj commented 5 years ago

Correct. I'll be working with Mozilla to getting that fixed once for good, but it will take some time.

adnux commented 5 years ago

Correct. I'll be working with Mozilla to getting that fixed once for good, but it will take some time.

Hi! Any updates on this? I was trying to make "react-pdf" 4.0.0 to work with "react-scripts" v2.1.2 but still having some trouble:

mikemclin commented 5 years ago

@adnux If you are upgrading, then your old implementation will not work. You will need to follow the Browserify & Others documentation.

Your imports/setup will probably look something like this...

import { pdfjs, Document, Page } from 'react-pdf';

pdfjs.GlobalWorkerOptions.workerSrc = `//cdnjs.cloudflare.com/ajax/libs/pdf.js/${pdfjs.version}/pdf.worker.js`;
rovaniemi commented 5 years ago

Hi! Any updates?

I still get this error message.

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

Environment

localjo commented 5 years ago

This warning seems to be related to Mozilla's PDF.js using require.ensure for fallback purposes, but create-react-app's webpack configuration warning about it since it's a non-standard language feature. I'm not sure what the correct solution is, but I've opened an issue on the create-react-app repo for discussion, and this issue on the PDF.js repo might also be relevant.

varunpatro commented 5 years ago

Is there any temporary fix to this problem at present?

maciejmatu commented 5 years ago

@varunpatro I am using craco to allow modification of CRA webpack config, and I remove the { parser: { requireEnsure: false } } rule. My issue was that the CI server would treat the warning as an error and I wasn't able to deploy the project. Remember it's a hacky temporary fix 😄

Here is craco.config.js code I use, maybe it will help someone. 

const { isEqual } = require("lodash");

module.exports = {
  webpack: {
    configure(webpackConfig) {
      const updatedRules = webpackConfig.module.rules.filter(
        rule => !isEqual(rule, { parser: { requireEnsure: false } })
      );
      webpackConfig.module.rules = updatedRules;

      return webpackConfig;
    }
  }
};
softwareplumber commented 5 years ago

Same CI issue as @maciejmatu - I will look into his hack but the person who fixes this properly will have my undying gratitude.

OnimeNoKyo commented 5 years ago

Same CI issue as @maciejmat

mbaric1 commented 5 years ago 
Compiled with warnings.

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.

Environment

@maciejmatu Hack works. But are there any other solutions or updates on this issue?

Thanks!

rsmything commented 5 years ago

Hello, the same issue react-scripts@2.1.8 pdfjs-dist@2.1.266 react-pdf@4.0.5

any updates?

akuji1993 commented 5 years ago

This is holding us back actually using your library. It means that in most cases, the CI software will not let this through, as warnings are interpreted as errors. And for a lot of people, @maciejmatu's solution won't work because we don't want to work around CRA default scripts. So this means either dropping react-pdf or finding some kind of a solution.. Which neither really is to my liking :/

mikemclin commented 5 years ago

@akuji1993 You can also disable warnings in your CI (which cause broken builds) by doing something like this: CI=false npm run build.

simoneb commented 5 years ago

@mikemclin that's generally not a good idea because quite a few tools behave differently when running under CI

mikemclin commented 5 years ago

@simoneb you only need to turn the CI off when running your Create React App build command. You could keep it on for other build steps (testing, deploy, etc).

softwareplumber commented 5 years ago

Per

https://github.com/facebook/create-react-app/issues/3657

I think the CI env var controls a couple of other things. Notably I think if you are running tests you have a catch-22 where you either see warnings as errors or you have the tests running in watch mode.

I tried to get the create-react-app folks to look again at controlling all that behavior with one flag, but no dice.

I’m using craco to work around this by hacking the standard CRA scripts without ejecting, and this seems to work for me at the moment, but I basically live in fear of having to update CRA.

Would still very much appreciate an actual fix for the warning…

Regards Jon

On Apr 29, 2019, at 10:19 AM, Mike McLin notifications@github.com wrote:

@simoneb https://github.com/simoneb you only need to turn the CI off when running your Create React App build command. You could keep it on for other build steps (testing, deploy, etc).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/wojtekmaj/react-pdf/issues/280#issuecomment-487598478, or mute the thread https://github.com/notifications/unsubscribe-auth/AGAPFY6ZFBI5PSCLUTL3UWTPS37XBANCNFSM4FYNCG2Q.

akuji1993 commented 5 years ago

We caved, asked around at other teams and went with https://github.com/timarney/react-app-rewired instead, also working with pretty much the same workaround that @maciejmatu provided. Also would still like to have an actual solution, but to be fair, that seems to lie more with Mozilla then with @wojtekmaj... So it will probably take forever.

Also disabling the CI flag is absolute nono for us.

jguitard commented 5 years ago 
Compiled with warnings.

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.

Environment

Removing the following code block from build/pdf.js suppresses the warnings, still keeping functionality in my case (I'm using external worker):

{
  var useRequireEnsure = false;

  if (typeof window === 'undefined') {
    isWorkerDisabled = true;

    if (typeof require.ensure === 'undefined') {
      require.ensure = require('node-ensure');
    }

    useRequireEnsure = true;
  } else if (typeof require !== 'undefined' && typeof require.ensure === 'function') {
    useRequireEnsure = true;
  }

  if (typeof requirejs !== 'undefined' && requirejs.toUrl) {
    fallbackWorkerSrc = requirejs.toUrl('pdfjs-dist/build/pdf.worker.js');
  }

  var dynamicLoaderSupported = typeof requirejs !== 'undefined' && requirejs.load;
  fakeWorkerFilesLoader = useRequireEnsure ? function () {
    return new Promise(function (resolve, reject) {
      require.ensure([], function () {
        try {
          var worker;
          worker = require('./pdf.worker.js');
          resolve(worker.WorkerMessageHandler);
        } catch (ex) {
          reject(ex);
        }
      }, reject, 'pdfjsWorker');
    });
  } : dynamicLoaderSupported ? function () {
    return new Promise(function (resolve, reject) {
      requirejs(['pdfjs-dist/build/pdf.worker'], function (worker) {
        try {
          resolve(worker.WorkerMessageHandler);
        } catch (ex) {
          reject(ex);
        }
      }, reject);
    });
  } : null;

  if (!fallbackWorkerSrc && (typeof document === "undefined" ? "undefined" : _typeof(document)) === 'object' && 'currentScript' in document) {
    var pdfjsFilePath = document.currentScript && document.currentScript.src;

    if (pdfjsFilePath) {
      fallbackWorkerSrc = pdfjsFilePath.replace(/(\.(?:min\.)?js)(\?.*)?$/i, '.worker$1$2');
    }
  }
}
obuw commented 5 years ago

I am also having this issue, it is getting rather frustrating.

I tried to use /* eslint-disable */ at the top of the build/pdf.js file but that got completely ignored.

I tried removing the code block that @jguitard mentioned, but that didn't change anything either.

I even tried removing the pdfjs-dist folder altogether, but somehow the project continues to work, and gives the same warnings, referencing files that don't even exist!

I don't know what kind of black magic is going on behind the scenes. But I am using an external worker: I have pdf.worker.min.js in my public folder. And I'm using this snippet:

import { Document, Page, pdfjs } from 'react-pdf'
pdfjs.GlobalWorkerOptions.workerSrc = 'pdf.worker.min.js';

Any suggestions would be appreciated.

dkrefta commented 5 years ago

Having this same issue here and my pipeline goes crazy with this error :(

akuji1993 commented 5 years ago

@dkrefta We’re using @maciejmatu solution with React-app-rewired and so far didn’t have any issue with it. If that’s helps...

dkrefta commented 5 years ago

@akuji1993 yeah, I know that but I had some problems with my pipeline but now I'm using unset CI hahaha and seems ok

gaPanza commented 5 years ago

Hello, the same issue react-scripts@2.1.5 pdfjs-dist@2.1.266 react-pdf@4.1.0

any news?

kivervinicius commented 5 years ago

same here...

webdevbyjoss commented 5 years ago

Also getting the warning that fails my CI build.

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

react-scripts@3.0.1 pdfjs-dist@2.1.266 react-pdf@4.1.0

Let me summarize the solutions we have so far:

  1. Run build with CI=false npm run build (loos like the easiest thing to workaround this, but I REALLY don't like it)
  2. Eject and remove { parser: { requireEnsure: false } } from config.
  3. Use craco and rewired to modify config without eject, and apply change described in item # 2
akuji1993 commented 5 years ago

If you want to fix this via react-app-rewired and without a CI=false command, insert the following code into config-overrides.js: 

const { isEqual } = require("lodash");

module.exports = function override(config, env) {
  // Ensures react-pdf doesn't throw a warning
  const updatedRules = config.module.rules.filter(
    rule => !isEqual(rule, { parser: { requireEnsure: false } })
  );
  config.module.rules = updatedRules;

  // Ensure jest configuration works
  config.collectCoverageFrom = [
    "src/components/**/*.{ts,tsx}",
    "!src/*.{ts, tsx}"
  ];
  return config;
};

This has worked for us without an issue now.

hugonasciutti commented 5 years ago

Using config-overrides.js with react-app-rewired to override create-react-app webpack configs without ejecting:

In react-scripts v3.1.1 requireEnsure rule is at position 0, you can change it directly.

module.exports = function override(config) {
  config.module.rules[0].parser.requireEnsure = true
  return config
}
llamamoray commented 5 years ago

Using config-overrides.js :

In react-scripts v3.1.1 requireEnsure rule is at position 0, you can change it directly.

module.exports = function override(config) {
  config.module.rules[0].parser.requireEnsure = true
  return config
}

@hugonasciutti is this a standard feature in CRA or is this using react-app-rewired?

hugonasciutti commented 5 years ago

@llamamoray using react-app-rewired. config-overrides.js file is to override create-react-app webpack configs without ejecting.

I edited my comment to be more complete.

frontr-uk commented 5 years ago

I get : referenceError: window is not defined

taddj commented 5 years ago

Ran into same issue,

./node_modules/pdfjs-dist/build/pdf.js Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

moved to 4.0.0 from 4.1.0 but error stayed the same

I noticed some solutions include config-overrides.js my current config-overrides.js is

const { override, addDecoratorsLegacy } = require("customize-cra");

module.exports = override(
  addDecoratorsLegacy(),
);

how can I combine this with the fix?

user7466 commented 5 years ago

Unless I am reading this wrong, this other react project using pdf-js has resolved this issue: https://github.com/mikecousins/react-pdf-js/releases/tag/v5.1.0 "Moving to the es6 bundled version of pdfjs-dist (@bundled-es-modules/pdfjs-dist) to get rid of the warnings when using Create React App." Could the same thing be used here?

frontr-uk commented 5 years ago

@taddj here is the thing, I spent a lot of hrs to understand what is going on as understanding docs it, not my thing.... after looking into the function 'config-overrides' you need to assign 'self' to the output.... here the code basically.

Most of the guys forget to say about this little magic line 'config.output.globalObject = 'this''

  1. look on the repo of https://github.com/timarney/react-app-rewired to understand how to install it, create the file and where to put the file

  2. Past the code and comment 👍 

module.exports = function override(config, env) {
  config.output.globalObject = 'this'
  config.module.rules[0].parser.requireEnsure = true

  return config;
};
taddj commented 5 years ago

@taddj here is the thing, I spent a lot of hrs to understand what is going on as understanding docs it, not my thing.... after looking into the function 'config-overrides' you need to assign 'self' to the output.... here the code basically.

Most of the guys forget to say about this little magic line 'config.output.globalObject = 'this''

  1. look on the repo of https://github.com/timarney/react-app-rewired to understand how to install it, create the file and where to put the file
  2. Past the code and comment 👍
module.exports = function override(config, env) {
  config.output.globalObject = 'this'
  config.module.rules[0].parser.requireEnsure = true

  return config;
};

Thank you for your reply.

My current config is 

const { override, addDecoratorsLegacy } = require("customize-cra");

module.exports = override(
  addDecoratorsLegacy(),
);

How do I modify my current config override to work with what you showed? I still need the DecoratorsLegacy

taddj commented 5 years ago

Resolved the issue as described here https://github.com/arackaf/customize-cra/issues/159

neomib commented 5 years ago 
Compiled with warnings.

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

./node_modules/pdfjs-dist/build/pdf.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.

Environment

  • React-PDF: 4.0.5
  • React: 16.8.6
  • Webpack: 4.29.6
  • React scripts: 3.0.0
  • Other: pdfjs.GlobalWorkerOptions.workerSrc = `//cdnjs.cloudflare.com/ajax/libs/pdf.js/${pdfjs.version}/pdf.worker.js`;

Removing the following code block from build/pdf.js suppresses the warnings, still keeping functionality in my case (I'm using external worker):

{
  var useRequireEnsure = false;

  if (typeof window === 'undefined') {
    isWorkerDisabled = true;

    if (typeof require.ensure === 'undefined') {
      require.ensure = require('node-ensure');
    }

    useRequireEnsure = true;
  } else if (typeof require !== 'undefined' && typeof require.ensure === 'function') {
    useRequireEnsure = true;
  }

  if (typeof requirejs !== 'undefined' && requirejs.toUrl) {
    fallbackWorkerSrc = requirejs.toUrl('pdfjs-dist/build/pdf.worker.js');
  }

  var dynamicLoaderSupported = typeof requirejs !== 'undefined' && requirejs.load;
  fakeWorkerFilesLoader = useRequireEnsure ? function () {
    return new Promise(function (resolve, reject) {
      require.ensure([], function () {
        try {
          var worker;
          worker = require('./pdf.worker.js');
          resolve(worker.WorkerMessageHandler);
        } catch (ex) {
          reject(ex);
        }
      }, reject, 'pdfjsWorker');
    });
  } : dynamicLoaderSupported ? function () {
    return new Promise(function (resolve, reject) {
      requirejs(['pdfjs-dist/build/pdf.worker'], function (worker) {
        try {
          resolve(worker.WorkerMessageHandler);
        } catch (ex) {
          reject(ex);
        }
      }, reject);
    });
  } : null;

  if (!fallbackWorkerSrc && (typeof document === "undefined" ? "undefined" : _typeof(document)) === 'object' && 'currentScript' in document) {
    var pdfjsFilePath = document.currentScript && document.currentScript.src;

    if (pdfjsFilePath) {
      fallbackWorkerSrc = pdfjsFilePath.replace(/(\.(?:min\.)?js)(\?.*)?$/i, '.worker$1$2');
    }
  }
}

@jguitard By (I'm using external worker): you mean this one : pdfjs.GlobalWorkerOptions.workerSrc = `//cdnjs.cloudflare.com/ajax/libs/pdf.js/${pdfjs.version}/pdf.worker.js`; ?

wojtekmaj commented 4 years ago

Good news everyone. This issue seem to have been fixed in https://github.com/mozilla/pdf.js/commit/d370037618211c8642dd3c576edbc2d2b3d6e09e! This means that as soon as the next stable release of pdf.js is out, we'll be able to resolve this issue.

hello-astar commented 4 years ago

@varunpatro I am using craco to allow modification of CRA webpack config, and I remove the { parser: { requireEnsure: false } } rule. My issue was that the CI server would treat the warning as an error and I wasn't able to deploy the project. Remember it's a hacky temporary fix 😄

Here is craco.config.js code I use, maybe it will help someone.

const { isEqual } = require("lodash");

module.exports = {
  webpack: {
    configure(webpackConfig) {
      const updatedRules = webpackConfig.module.rules.filter(
        rule => !isEqual(rule, { parser: { requireEnsure: false } })
      );
      webpackConfig.module.rules = updatedRules;

      return webpackConfig;
    }
  }
};

thanks, it works!!!

VinhVu0412 commented 4 years ago

We can use CDN instead. Do we have react-pdf.min.js?

AprilPolubiec commented 4 years ago

If you want to fix this via react-app-rewired and without a CI=false command, insert the following code into config-overrides.js:

const { isEqual } = require("lodash");

module.exports = function override(config, env) {
  // Ensures react-pdf doesn't throw a warning
  const updatedRules = config.module.rules.filter(
    rule => !isEqual(rule, { parser: { requireEnsure: false } })
  );
  config.module.rules = updatedRules;

  // Ensure jest configuration works
  config.collectCoverageFrom = [
    "src/components/**/*.{ts,tsx}",
    "!src/*.{ts, tsx}"
  ];
  return config;
};

This has worked for us without an issue now.

This worked for me, but removed this section:

   config.collectCoverageFrom = [
     "src/components/**/*.{ts,tsx}",
     "!src/*.{ts, tsx}"
   ];

Thanks!

bugzpodder commented 4 years ago

@wojtekmaj https://github.com/mozilla/pdf.js/releases/tag/v2.4.456 was just released

VinhVu0412 commented 4 years ago

The react-pdf 4.1.0 doesn't work with pdf.js 2.4.456. 😢