-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
base: master
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for ohif-platform-docs failed. Why did it fail? →
|
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -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}');`, |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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.
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. |
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. |
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
etc.)
Public Documentation Updates
additions or removals.
Tested Environment