-
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
Support Systemjs #6791
Comments
I would request more information since no examples are provided. And I think Browserify works with pdfjs-dist currently (with some modules exceptions). |
Here's the most common example to use systemjs:
index.html
src/app.js
|
Thank you for example |
In previous versions we could use jspm shim to use pdfjs. |
Actually, this is a request for jspm packager, not specifically Systemjs loader. |
Having the following in the config.js workaround the issue:
(empty is a empty.js file with no content) Similar hack is need for Browserify |
Now issue: pdf.worker.js is loaded twice, that's an issue was solved by request.ensure() in webpack. The './pdf.worker.js' has to be forbidden from loading until it's really needed. |
I thought "PDFJS.disableWorker = true;" solves that? Isn't it? |
If you use Or even Is there a specific reason why |
"PDFJS.disableWorker = true;" is not an option,since we want to move PDF processing on the different thread. We don't want suggest lousy solution for library users, do we? |
Well, I'd be prefer lousy solution than no solution at all. For my use case, even 1 second page freeze is fine. |
If you look carefully at what examples/webpack/ produces:
"bundle.js" is main file that loads (that means it will bring and initialize UI after 122796 bytes -- faster page start up). "9d074593b165291f150e-worker.js" is a worker which will run the logic and don't lock UI on slow devices. "1.bundle.js" portion is loaded only if disableWorker triggered and for legacy browsers such as IE9. |
I'm expecting the systemjs might have some kinda guard to not parse all requires (replacing "./pdf.worker.js" with "empty" did not work for me). @Vanuan Can you file an issue with systemjs to see if they have something better in mind? Thanks. |
Sounds like the very last issue at systemjs systemjs/systemjs#983 ? |
@Vanuan, nothing changed in usage of the pdf.combined.js file (I hope) -- looks like we need to keep it for such cases. You need to use it as you described as an issue in #6729. From your description it was not clear you are planing to use systemjs to load PDFJS. |
Ah, great! |
Anyway, something has definitely changed with regard to using it with worker.
Now, it seems like it's stuck on loading files. As if module loaded callback never completes. Probably, Systemjs can't process the following construction:
|
pdf.combined.js fully included/includes pdf.worker.js and does not need workerSrc to be specified. |
Changes to pdf.combined.js mozilla/pdfjs-dist@a7cd5f7#diff-eccf5b94e31b0939738de07167e02af6 |
Yeap, I'm just evaluating options of using workers along with jspm. |
So, now it is being loaded in the main thread and doesn't use importScripts()? |
My src/app.js is:
So it's not even using getDocument yet, but in network monitor it shows two entries for pdf.worker.js |
It's strange. I only see one entry. |
It shall be none AFAIK :) |
Ah. So looks like Systemjs dumbly parses all |
Conditional require is not a very common use case. |
That's why it tries to load node-ensure too. |
@yurydelendik |
I would disagree, systemjs/systemjs#983 and https://github.com/webpack/docs/wiki/code-splitting are common use cases.
|
The issue will be solved by #6775 -- the systemjs has auto-detect for type of the module. Now it's commonjs, with UMD it will be AMD so it will not try to parse require(). P.S. wip at https://github.com/yurydelendik/pdf.js/tree/pdfjsumd |
Wow, great, I'll try as soon as merged and released. |
Closing as fixed by #6825. If there are remaining problems, please open a new issue. |
I also recommend to override module format when using jspm: |
Please add support for systemjs.
The text was updated successfully, but these errors were encountered: