-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Add optional argument to PDFWorker for custom/bundled workers #7683
Conversation
- Support for webpack, browserify, etc users to pass in the proper web worker - E.g. webpack's worker-loader, browserify's webworkify
If this is accepted, we might need to update the examples in the |
Thank you for request. It is really close to the desirable solution. The customWorker does not need to be tested, this also mean we cannot really fallback to fake worker (we will not know how to get to the js code to init that) -- all responsibility of giving proper port shall fall on the shoulders of the user/consumer. Let's rename customWorker to customPort and avoid all byte transfer testing -- just "jump" to the MessageHandler initialization. That said: a worker's port shall be ready to accept "GetDocRequest" without test/ready conversation. |
@yurydelendik doing that would require putting several if statements around the code and abstracting functions to be able to call them from two different places (or duplicating code), at which point it would indeed be easier to write a totally separate constructor. Several functions would still need to be abstracted out of the initialize code in order to be used in both constructors. It took me a good hour just to figure that out and the several binds make it even more confusing what the proper scope should be if functions are abstracted out (not to mention I have no idea what to do with the various Correct me if I'm wrong, but it doesn't seem like the PR itself has any bugs in it, just that it could be optimized. My users and I are experiencing significant lag in reading PDFs due to #7612 and have been for the past 4 weeks (and I'm not the only one facing this bug), so I would sincerely appreciate if a fix could be released ASAP. As such, do you think we could get this PR merged first as it fixes the major performance bug in #7612? I can make the changes to Please let me know if that's possible and thank you for your understanding. |
Any progress on this? |
+1 for this issue. Not sure why this wasn't designed this way anyway. I'm having an issue where I can't set the workerSrc at all because the The offending line is here: Line 99 in cfaa621
|
Closing since it's superseded by #8107. |
Meant as a prototype to fix #7612 . This isn't quite implemented as an overloaded constructor but instead with an optional argument.
An update to the wiki documenting how to use this functionality should be followed up.