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

Import Path Dicom Microscopy viewer #4902

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

salimkanoun
Copy link
Contributor

@salimkanoun salimkanoun commented Mar 28, 2025

Context

Following #4891 trying to solve the import of dicom microscopy viewer if served by non root path.

When using PUBLIC_URL, dicom-microscopy-viewer retrieve from OHIF still miss the basename prefix,

Changes & Results

In importPath of pluginConfig was settings "/dicom-microscopy-viewer/dicomMicroscopyViewer.min.js " so was evaluated as absolute path and thus PUBLIC_URL was not applyed in write pluginImportFile.js

Removing the first '/' to make it relative solves this issue for retrieving the assets but OHIF crashes on another issue by calling
"/dicom-microscopy-viewer/index.worker.min.worker.js" from an absolute path without adding the PUBLIC_URL in this http request (but didn't found where it comes from)

Testing

Use a build with PUBLIC_URL, with this fix retrieve of dicomMicroscopyViewer.min.js is OK but fails for index.worker.min.worker.js

Checklist

PR

Code

  • [] My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

Copy link

netlify bot commented Mar 28, 2025

Deploy Preview for ohif-platform-docs failed. Why did it fail? →

Name Link
🔨 Latest commit a1d4766
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/67e721dc7be06d000834e957

Copy link

netlify bot commented Mar 28, 2025

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit a1d4766
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/67e721dc6c204a00081d02a9
😎 Deploy Preview https://deploy-preview-4902--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sedghi sedghi requested a review from wayfarer3130 April 3, 2025 13:12
@@ -77,7 +73,7 @@ function getRuntimeLoadModesExtensions(modules) {
if (module.importPath) {
dynamicLoad.push(
` if( module==="${packageName}") {`,
` const imported = await window.browserImportFunction('${isAbsolutePath(module.importPath) ? '' : publicURL}${module.importPath}');`,
` const imported = await window.browserImportFunction('${publicURL}${module.importPath}');`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't undo the change - if you want to prefix it, then use a relative path, and if you don't, then use an absolute path. This change will break absolute paths to external modules.

@@ -96,7 +96,7 @@
},
{
"packageName": "dicom-microscopy-viewer",
"importPath": "/dicom-microscopy-viewer/dicomMicroscopyViewer.min.js",
"importPath": "dicom-microscopy-viewer/dicomMicroscopyViewer.min.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change by itself is sufficient to load the dicom microscopy viewer inside the publicURL path, without the change to writePluginImportsFile change. However, the library itself can't handle being loaded at a relative path.

@wayfarer3130
Copy link
Contributor

Removing the first '/' to make it relative solves this issue for retrieving the assets but OHIF crashes on another issue by calling
"/dicom-microscopy-viewer/index.worker.min.worker.js" from an absolute path without adding the PUBLIC_URL in this http
request (but didn't found where it comes from)

The problem is an internal issue in dicom-microscopy-viewer libraries that has to be solved in that library. Without fixing that internal issue, changing to a relative path is insufficient.

@wayfarer3130
Copy link
Contributor

To get the dicom microscopy viewer loading with a public url, you will have to publish it on the root path, OR create a custom version of the microscopy viewer that knows about the public path. This didn't work in 3.9 either with a public path and not serving it on the root path as it is a limitation of the internal library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants