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

PDFScriptLoaderService fails to properly load the pdfjs viewer after destroyed. #2510

Open
Hypercubed opened this issue Aug 16, 2024 · 9 comments
Assignees
Labels
21 bug Something isn't working

Comments

@Hypercubed
Copy link

Here is a tricky one.... I noticed that when running >v21, component tests started breaking. After quick a bit of investigation I'm pretty sure I've narrowed down the issue to the changes introduced in 21.0.0-alpha.0. Here is the breakdown:

21.0.0-alpha.0 introduces PDFScriptLoaderService which loads the pdfjs viewer inside the loadViewer method. The loadViewer method relies on the ngxViewerFileHasBeenLoaded event being triggered when loading the viewer mjs file. This works fine on initial load since the mjs file includes the event trigger. However, in unit/component testing the Angular app is destroyed and the PDFScriptLoaderService#destroy method is called to cleanup. The Angular app is then "rebooted" in the same window. But since the viewer is loaded as a module, it is not executed again (ES modules are loaded once and reused). The ngxViewerFileHasBeenLoaded event is never triggered and therefore the pdfjs is never initailzed.

Seams like we need a different way to trigger the listener inside loadViewer that doesn't rely on side effects inside the module.

I've only seen this in testing; since a app usually has the same lifecycle as the document/window.

@stephanrauh stephanrauh self-assigned this Aug 16, 2024
@stephanrauh
Copy link
Owner

Tricky indeed. To be honest, I'm a clueless.

@stephanrauh stephanrauh added bug Something isn't working 21 labels Aug 16, 2024
@Hypercubed
Copy link
Author

Esiest fix I can up with is to cachebust the viewer mjs file. Two lines changed:

const viewerPath = this.getPdfJsPath('viewer') + '?v=' + new Date().getTime();

and in createScriptElement

script.type = sourcePath.includes('.mjs') ? 'module' : 'text/javascript';

Not the best solutions... since it also bypasses the browser cache.

@Hypercubed
Copy link
Author

Guessing the better fix is to wrap these lines in a global function that gets called onload. https://github.com/stephanrauh/ngx-extended-pdf-viewer/blob/main/projects/ngx-extended-pdf-viewer/assets/viewer-4.5.713.mjs#L38804C1-L38812C31

@Hypercubed
Copy link
Author

Hypercubed commented Aug 16, 2024

Something like this (untested):

DOESN'T WORK

  function ngxViewerFileHasBeenLoaded() {
    const event = new CustomEvent('ngxViewerFileHasBeenLoaded', {
      detail: {
        PDFViewerApplication: __webpack_exports__.PDFViewerApplication,
        PDFViewerApplicationConstants: __webpack_exports__.PDFViewerApplicationConstants,
        PDFViewerApplicationOptions: __webpack_exports__.PDFViewerApplicationOptions,
        webViewerLoad: __webpack_exports__.webViewerLoad,
      },
    });
    document.dispatchEvent(event);
  }

  if (document.readyState === "interactive" || document.readyState === "complete") {
    ngxViewerFileHasBeenLoaded();
  } else {
    document.addEventListener("DOMContentLoaded", ngxViewerFileHasBeenLoaded, true);
  }

@Hypercubed
Copy link
Author

OK.. Here is the best I got:

In viewer.mjs:

  function ngxViewerFileHasBeenLoaded() {
    const event = new CustomEvent('ngxViewerFileHasBeenLoaded', {
      detail: {
        PDFViewerApplication: __webpack_exports__.PDFViewerApplication,
        PDFViewerApplicationConstants: __webpack_exports__.PDFViewerApplicationConstants,
        PDFViewerApplicationOptions: __webpack_exports__.PDFViewerApplicationOptions,
        webViewerLoad: __webpack_exports__.webViewerLoad,
      },
    });
    document.dispatchEvent(event);
  }

  window.ngxViewerFileHasBeenLoaded = ngxViewerFileHasBeenLoaded;

In pdf-script-loader.service.ts#loadViewer:

            document.addEventListener('ngxViewerFileHasBeenLoaded', listener);
            const script = this.createScriptElement(viewerPath);
            script.onload = () => {
                window.ngxViewerFileHasBeenLoaded();
            };
            document.getElementsByTagName('head')[0].appendChild(script);

@Hypercubed
Copy link
Author

Could bypass the ngxViewerFileHasBeenLoaded altogether if ngxViewerFileHasBeenLoaded() returns the details directly.

I can do a PR if you are happy with having ngxViewerFileHasBeenLoaded as a global.

@stephanrauh
Copy link
Owner

stephanrauh commented Aug 17, 2024

I think we need a solution that does not pollute the global namespace. The best solution would be to wrap the viewer file in an Angular component. Unfortunately, I haven't managed to do that yet.

In the long run, I'd like to be able to have multiple independent instances of the viewer simultaneously. It seems many developers need this feature, and https://pdfviewer.net/extended-pdf-viewer/side-by-side uses a very clumsy work-around.

@stephanrauh
Copy link
Owner

@Hypercubed What's the current state of the art? Do you expect me to do something, or do you want to send a pull request?

Oh, and I wish you a Happy New Year!

@Hypercubed
Copy link
Author

Happy new year to you!

I haven't look at this in a little while. I belive I had a not so great solution... I'll need to look again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
21 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants