stephanrauh / ngx-extended-pdf-viewer

A full-blown PDF viewer for Angular 16, 17, and beyond
https://pdfviewer.net
Apache License 2.0
484 stars 184 forks source link

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

Open Hypercubed opened 2 months ago

Hypercubed commented 2 months ago

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 commented 2 months ago

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

Hypercubed commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

~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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.