Review / test CommonJS build generated by Typescript

[demiankatz]

Just as #1502 proposes testing and evaluating the ESM build, we should do the same for the CommonJS build. We should review it and make sure that it works correctly. If there is a way to automatically test it, we should consider adding more build testing to our continuous integration processes to prevent future regressions. If it is no longer used or needed, we should consider simplifying our build process.

[Geoffsc]

@demiankatz I looked at the examples but didn't see one for CommonJS. I installed electron to do an extremely basic test which really only verifies that the CommonJS bundle at dist/cjs/index.js can be loaded via require(). This was successful, but that was the extent of my testing. I agree that if it's not being used/isn't required, it would be good to simplify the build!

[demiankatz]

Thanks, @Geoffsc! I wonder if it's possible to make a small Electron app that actually displays UV, similar to the Vite example at https://github.com/UniversalViewer/example-vite. If we can make that work, and if it builds using CJS, that would solve our remaining testing issues. Not sure if it's possible, though -- any thoughts? Worth a workshop to try?

[Geoffsc]

@demiankatz Electron/CJS example is now created here. If you or @jamesmisson or someone else could review, I would appreciate it!

[demiankatz]

Thanks, @Geoffsc! Is there an easy way to parameterize an Electron app? It would be nice if there was a way to specify a manifest and have it load without having to change the code. If you're not sure, I can look into it when time permits. In any case, I see you had to do some workarounds to make OSD work right, so it's definitely worth testing PDF and media manifests to see if they break or not -- and that would be easier if we could just provide a URL as an argument without having to rebuild everything.

[Geoffsc]

@demiankatz I just updated the repo to parameterize the manifest url, I agree other manifests should definitely be tested!

[demiankatz]

@demiankatz I just updated the repo to parameterize the manifest url, I agree other manifests should definitely be tested!

Thanks, @Geoffsc, that's very helpful!

Unfortunately, this reveals that both the MediaElement and PDF extensions do not work in this context. I have opened issues on the example repo to track these problems individually. I've also opened a PR to address some NPM warnings I saw during testing.