Pdf.js: Upgrade `systemjs` to the most recent version

[timvandermeij]

Currently we use the systemjs 0.x version, but the most recent version as of writing is the 4.x version. Updating is a bit difficult because of API changes, but should in return simplify the logic and also allow us to e.g., retry #10953 afterwards since that it blocked on this update.

More information can be found on https://guybedford.com/systemjs-2.0.

[Samrose-Ahmed]

I'd like to take this.

[timvandermeij]

Sure, please do!

[arvind0598]

If this is still inactive, can I try my hand at this?

[timvandermeij]

We're one month further and we haven't seen any activity on this, so feel free to work on this!

[Snuffleupagus]

For anyone working on this, please make sure to carefully test your patch prior to submitting a PR. This entails, at the very least:

• Ensure that the development viewer works correctly, by running gulp server and:
  
  
  
  • Open the development viewer, with http://localhost:8888/web/viewer.html, in a browser and make sure that the viewer both loads and works correctly without any Errors in the console.
  
  
  • Run the unit-tests directly in a browser, with http://localhost:8888/test/unit/unit_test.html, and ensure that all the same tests pass with the patch as with the master branch.
  
  
• Run gulp unittest locally, and ensure that all tests pass.
• Run gulp fonttest locally, and ensure that all tests pass.
• Run gulp browsertest locally, and ensure that all tests pass.
• Finally run gulp test locally, and ensure that all tests pass.

Useful resources:

• https://github.com/mozilla/pdf.js#getting-the-code
• https://github.com/mozilla/pdf.js/wiki/Contributing

[arvind0598]

I need some help with gulp browsertest. When I run it, there are a lot of outputs that say WARNING: no reference snapshot ref/linux/Firefox/issue8229/1.png, or something similar.

Then the following happens, and the test fails with 546 errors. I'm not sure how to get this working so that I can start working myself. unittest and fonttest pass completely.

TEST-UNEXPECTED-FAIL | test failed Chrome has not responded in 120s
TEST-UNEXPECTED-FAIL | test failed Firefox has not responded in 120s

I'm running Firefox 68.0.2 and Chromium 76.0.3809.100-0ubuntu0.18.04.1 on Ubuntu 18.04.3

[Snuffleupagus]

I need some help with gulp browsertest. When I run it, there are a lot of outputs that say WARNING: no reference snapshot ref/linux/Firefox/issue8229/1.png, or something similar.

This would suggest that you didn't read/follow https://github.com/mozilla/pdf.js/wiki/Contributing as mention above, since the heading "Generating reference images" (under https://github.com/mozilla/pdf.js/wiki/Contributing#4-run-lint-and-testing) outlines the necessary steps.

[MrNoddyInfy]

@arvind0598 are you still working on this issue?

[josephlane]

@MrNoddyInfy I didn’t see a response for over 2 months for this issue so on Monday, I attempted to see if I could quickly resolve in the background. Here’s where I am as of now. I’ve updated SystemJS to the latest version as of this current writing. It’s a breaking change, so I’m parsing through SystemJS documentation to convert the old system.config code to use import maps. My plan was to have this resolved by next week. If you feel you can resolve sooner, I wouldn’t have a problem finding another issue.

[MrNoddyInfy]

Thanks for update @josephlane, I think it's good you continue on this issue, as you have started already. I will look for some other issue.

[josephlane]

@timvandermeij @Snuffleupagus

Hello All. I just wanted to provide an update on where I currently stand with this and see if someone can provide a little direction.

I pulled down the latest code and confirmed that I could execute and pass all unit tests.

I then upgraded SystemJS to version to 6.1.4 and converted the systemjs.config.js config to use import maps.

The paths config from below:

SystemJS.config({ packages: { '': { defaultExtension: 'js', }, }, paths: { 'pdfjs': new URL('src', baseLocation).href, 'pdfjs-web': new URL('web', baseLocation).href, 'pdfjs-test': new URL('test', baseLocation).href, 'pdfjs-lib': new URL('src/pdf', baseLocation).href, 'core-js': new URL('node_modules/core-js', baseLocation).href, 'web-streams-polyfill': new URL('node_modules/web-streams-polyfill', baseLocation).href, }, meta: { '*': { scriptLoad: false, esModule: true, babelOptions: { env: false, plugins: [babelPluginReplaceNonWebPackRequire], }, }, }, map: { 'plugin-babel': new URL(PluginBabelPath, baseLocation).href, 'systemjs-babel-build': new URL(SystemJSPluginBabelPath, baseLocation).href, 'plugin-babel-cached': new URL(PluginBabelCachePath, baseLocation).href, }, transpiler: isCachingPossible ? 'plugin-babel-cached' : 'plugin-babel', });

Was converted to the following:

<script type="systemjs-importmap"> { "imports": { "pdfjs/": "../../src/", "pdfjs-web/": "../../web/", "pdfjs-test/": "../../test/", "pdfjs-lib/": "../../src/pdf/", "core-js/": "../../node_modules/core-js/", "web-streams-polyfill/": "../../node_modules/web-streams-polyfill/" } } </script>

I'm currently receiving the below error message when attempting to execute unit tests:

Any ideas on what may be causing this issue? I'd be more than happy to keep digging but I feel as though I've hit a brick wall.

[Snuffleupagus]

I'm currently receiving the below error message when attempting to execute unit tests:

It's not clear here if you were able to successfully load the development viewer, as mention in the very first bullet point in https://github.com/mozilla/pdf.js/issues/10965#issuecomment-524825830, since if that's not working either that's probably where you'd need to start investigating.

For everything else though, you'll need to provide runnable code here since code snippets are unfortunately not actionable (since no one else can run your exact code).

[leoavelino]

@josephlane are you still working on this issue?

[josephlane]

@josephlane are you still working on this issue?

Feel free to work on this issue as I'm unsure of how to resolve the issues I was experiencing. I wasn't even able to load the development viewer as Snufflepagus suggested in earlier post.

@leoavelino

[Snuffleupagus]

The ideal solution here would probably be to eventually get rid of SystemJS completely, and instead use proper imports in non-PRODUCTION code, however that's currently blocked on Web Worker support for JavaScript modules; see bug 1247687 and bug 1540913.

[asingh04]

Hi, Is this issue open to be worked upon?
I would like to start from this one.

[timvandermeij]

Some work is currently in progress to reduce the dependency on/usage of SystemJS, so it might be easier to wait until that's done so the remaining upgrade work will be easier.

[Snuffleupagus]

To follow-up on https://github.com/mozilla/pdf.js/issues/10965#issuecomment-568252833, there's probably another potential solution that'd allow us to get rid of SystemJS immediately without having to wait for future worker modules support. Considering that even with those in place, unless something like https://wicg.github.io/import-maps/ is also available it's not clear to me if using native browser loading would even work correctly in all instances!?

The other solution, that we may want to look into, would be to actually use (a variant of) the "built" code during development. Essentially we should be able to use gulp watch to re-build the project automatically when files change.
This would have the advantage of local development/testing being a lot closer to what happens in "real" builds, but would unfortunately be slightly slower whenever the pdf.worker.js file needs to be re-built.
@timvandermeij Do you think this is something worth considering, as it seems unlikely that we'd be able to use a fully browser native solution any time soon?

[timvandermeij]

Yes, that sounds like a very interesting idea. I do wonder how much slower that would be given that the SystemJS solution also isn't particularly fast all the time, but I guess we can only tell if we implement this. Is it perhaps possible to make a lighter build, comparable to frameworks like Angular that offer dev and prod builds?

[Snuffleupagus]

I do wonder how much slower that would be given that the SystemJS solution also isn't particularly fast all the time,

Building only the pdf.worker.js file, and nothing else, takes consistently ~11 seconds for me locally.
(The other two files, i.e. pdf.js and viewer.js, are both smaller and consequently much faster.)