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

[Webpack] Chunk for worker file is not created #7165

Closed
redhead opened this issue Apr 7, 2016 · 16 comments
Closed

[Webpack] Chunk for worker file is not created #7165

redhead opened this issue Apr 7, 2016 · 16 comments

Comments

@redhead
Copy link

redhead commented Apr 7, 2016

When using webpack, the chunk that should be populated for the worker.js file does not exist and the dynamic load of the worker fails when pdfjs is used.

Also, when webpack finishes I have a chunk that contains the following:

webpackJsonp([1],{

/***/ 92:
/***/ function(module, exports) {

    /* (ignored) */

/***/ }

});

I have no idea, what that means.

The version 1.4.x has this problem, version 1.3.x works ok.

Note: we are using pdfjs-dist npm package.

@yurydelendik
Copy link
Contributor

regression from #7150, probably webpack is looking into "browser" section as well (?)

@yurydelendik
Copy link
Contributor

Looks like the complexity of the webpack solution does not let us to introduce browserify support and also complicated. We may need to redo the webpack the same way we support browserify and system.js.

@yurydelendik
Copy link
Contributor

@redhead
Copy link
Author

redhead commented Apr 12, 2016

I am maybe lost in the code of pdfjs but how does browser field relate to this bug? (Out of curiosity)

@eransagi1
Copy link

I also ran to this issue and able to workaround by pinning the version to 1.4.149.

@yurydelendik
Copy link
Contributor

@redhead right, now we are trying to support all available packagers (without any preference). Since there is no good support for our use case from any of frameworks vendors, I'm trying to add a workarounds. So far:

  • webpack requires to add "entry?.." loader and "node-ensure"
  • browserify does not support workers and barfs on "entry?name=[hash]-worker.js!./pdf.worker.js"
  • almond has its own quicks, see Support almond.js #7177 #7185

So far webpack solution looks most ugliest.

When we will find from defunctzombie/package-browser-field-spec#3 what is wrong browserify or webpack, we will make a decision to revert #7150 or make obsolete webpack support because its complexity. Please provide more information on the subject so we will make a right decision. Also find me on IRC irc.mozilla.org/pdfjs

@redhead
Copy link
Author

redhead commented Apr 12, 2016

Well, I merely want to use pdf.js to render pdfs in browser and we use webpack for bundling our application. Webpack was very helpful at not loading the pdf.js script when PDF rendering was not yet needed in our app. Just when the user wants to view a PDF, we dynamically require (using require.ensure) the 'pdf-dist' and then do stuff to render the file.

For sure it is because of the browser field, that you mentioned according to my tests.

I do not know how browserify works, but I doubt it should be resolved by the browser field. Because that is supposed to be working across bundlers (so if it disables loading for one, it disables loading for others).


I even tried to bundle and load the worker.js file my own way as:

require.ensure([], function () {
    var pdfjsLib = require('pdfjs-dist');
    pdfjsLib.PDFJS.workerSrc = require('file!pdfjs-dist/build/pdf.worker.js')
    pdfjsLib.getDocument(...);
            ...
})

Which should be good, but again it fails because of that browser field (if I remove it, it works). I cannot think of any other way of how to bundle the worker and load it using webpack, so it works.


I just removed completely webpack support from pdf.js and left it manual (setting workerSrc as above) and removed the lines from browser field, this is the most clean version of bundling I've yet got from webpack (in previous build, even on 1.3.x, it bundled the worker file twice for some reason, one of the files was never being loaded by any script).

@yurydelendik
Copy link
Contributor

it bundled the worker file twice for some reason, one of the files was never being loaded by any script).

Ideally it shall be bundled twice: one for use with worker and other one for PDFJS.disableWorker support (e.g. for IE9); or use the same bundle from two places. But as you said, the goal is the pdf.js shall not be loaded until it's needed and it is wrong if a packager includes pdf.worker code in the main bundle.

@redhead
Copy link
Author

redhead commented Apr 12, 2016

Oh I see the point with the 2 files (in case of IE9, pdfjs inserts a <script> with the worker file to the page?).

Webpack however does not bundle pdfjs (nor the worker) to the main bundle file but it creates two extra chunks (?) with the same worker code. One is loaded as a web worker, the other never I guess (I don't know how the IE9 support works if it uses the second file).

Anyway, in 1.3.x it worked as expected loading the files on-demand when needed. The current browser field disables that now though.

@yurydelendik
Copy link
Contributor

in case of IE9, pdfjs inserts a script with the worker file to the page?

That's correct. That's another bundle that is not-a-worker one.

The current browser field disables that now though.

Right, this field was introduced for browserify and that's needed for browserify to work (not include the worker code in the main bundle). But unfortunately webpack supporting it as well, and both of them in different way.

@chenzhutian
Copy link

Pin the version to 1.4.149 can workaround this issue.

@wmaca
Copy link

wmaca commented Apr 13, 2016

I am having the same issue here: worker file is ignored (worse than that, worker is an empty object). I thought it misusing webpack.

@yurydelendik
Copy link
Contributor

The worker is not bundled automatically after #7189. Please add workerSrc setting and bundling procedure for pdf.worker.entry.js. See https://github.com/mozilla/pdf.js/blob/master/examples/webpack/main.js#L11 and https://github.com/mozilla/pdf.js/blob/master/examples/webpack/webpack.config.js#L8 .

@andrewgleave
Copy link

@redhead Do you have async loading functional again after this change? I have pretty much the same configuration as you but can't get it to pull in the worker correctly.

Edit: Fixed

@redhead
Copy link
Author

redhead commented Apr 13, 2016

@andrewgleave Didn't try yet, but I think it should work since workerSrc is just a URL to the actual file which is loaded by pdfjs as worker dynamically. You just need to ensure webpack does not bundle the file into your bundle by using the file loader, for example, to get the file path only (and thus create a chunk out of that worker file).

@andrewgleave
Copy link

Yeah, I got it working. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants