OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.37k stars 3.4k forks source link

VTK Extension does not work as UMD module #1003

Closed swederik closed 1 year ago

swederik commented 5 years ago

If we use it as a script tag a runtime, it is not working:

https://codesandbox.io/s/ohif-script-tag-v144-runtime-extensions-okrdv

https://github.com/salimkanoun/GaelO/blob/dev/ohif/index.php#L62

Thanks @salimkanoun

dannyrb commented 5 years ago

I'm concerned we won't be able to get it to work unless we bundle vtk.js, or take an external approach like we do with the cornerstone libraries. Largely because vtk.js doesn't have "top level exports" for ES6 consumption, we're forced to pull in each piece we need from specific files under /Sources/

pieper commented 5 years ago

It's not really an extension if you need to bundle it... What is the external approach?

dannyrb commented 5 years ago

The extension doesn't need to be bundled into the app, the VTK.js dependency may need to be bundled with the extension.

If you want to consume vtk.js as an ES6 Dependency, you import and use it like so:

import vtkFullScreenRenderWindow from 'vtk.js/Sources/Rendering/Misc/FullScreenRenderWindow';
import vtkActor           from 'vtk.js/Sources/Rendering/Core/Actor';
import vtkCalculator      from 'vtk.js/Sources/Filters/General/Calculator';
import vtkConeSource      from 'vtk.js/Sources/Filters/Sources/ConeSource';
import vtkMapper          from 'vtk.js/Sources/Rendering/Core/Mapper';
import { AttributeTypes } from 'vtk.js/Sources/Common/DataModel/DataSetAttributes/Constants';
import { FieldDataTypes } from 'vtk.js/Sources/Common/DataModel/DataSet/Constants';

(This is the recommended approach in their documentation)

Because we're not importing everything from vtk.js, WebPack sees each of these as different entry points. Each one needs to be declared as a separate external in the WebPack configuration, or we need to use regex to exclude all of them. I haven't done much testing, but I believe this makes it difficult for the resulting package (the extension) to resolve these external dependencies for script-tag (UMD) usage.

For the PWA, we have access to /Sources/ at the time of bundling, so everything works as expected. For runtime extensions, I don't believe the extension's package is capable of resolving those dependencies from the platform.

If someone with more experience, or from Kitware, were to weigh in, I would be incredibly grateful, as I'm not 100% sure I know the best path forward here. The "just make it work" / easiest path forward here is to include vtk.js in the extension's package instead of externalizing.

pieper commented 5 years ago

Sorry to be dense here, but why does the extension need to be bundled? I think it's best to let it just pull in the code via the script tag when the extension is activated.

dannyrb commented 5 years ago

It's entirely possible I'm misunderstanding what you mean. Let me expand a bit:

For PWA? All extensions defined in the entry point are included in the bundling process.

For Script-Tag? We publish many "bundles" (packages):

Allowing for stuff like:

<div id="root"></div>

<script src="https://unpkg.com/@ohif/viewer@1.1.10/dist/index.umd.js"></script>
<script src="https://unpkg.com/@ohif/extension-dicom-microscopy@0.50.5/dist/index.umd.js"></script>
<script>
  var containerId = "root";
  OHIFViewer.installViewer(
    {
      servers: { dicomWeb: [{ /* Server Config */ }] },
      extensions: [OHIFExtDicomMicroscopy]
    },
    containerId,
  );
</script>

The crux of the problem, at least as far as I understand it, is how and where do we include vtk.js?

  1. With a script tag? I think because we import it like this, we are not able to:

import { FieldDataTypes } from 'vtk.js/Sources/Common/DataModel/DataSet/Constants';

  1. As a direct dependency of @ohif/viewers that is somehow magically resolved when the extension is added at load / runtime?

  2. Included in the @ohif/extension-vtk package? This is easiest, but could cause duplicate instances of VTK.js if another extension also depends on it.

I hope that makes sense. Please don't hesitate to call me out if I'm missing something obvious or misunderstanding the issue at hand.

pieper commented 5 years ago

Hi @dannyrb - when we were first talking about plugins I made this little example: https://github.com/pieper/hello-plugin

Other details aside, what this approach did was bring in the vtkjs via script tag after the viewer was already up and running rather than making it part of the config. That's the kind of dynamic behavior and modularity I was looking for.

salimkanoun commented 4 years ago

Dear all,

Sorry to disturb your hard work but Is there any new about this issue ? This one prevent me to upgrade OHIF in my project.

I was wondering, how do you make the https://viewer.ohif.org/ integration ?. Your demo page does not suffers from this bug as the VTK extension is working. Is there available source code from you demo page so I can learn how to escape this issue ?

Thanks again for your amazing work, I see OHIF getting so much better that i'm frustrated to be stuck on a old version ^^

Salim

dannyrb commented 4 years ago

@salimkanoun, you are always more than welcome to check-in and ask questions ^_^

https://viewer.ohif.org/ does not use the published @ohif/viewer that sits on npm. Your usage probably has an index.html file that uses that bundle in a single script include tag that points at something like: https://unpkg.com/@ohif/viewer

While that is handy for a quick setup, I'd likely steer most people creating a product or a dedicated instance to use the "Progress Web Application (PWA)" version of the OHIF Viewer. This version is generated when you use the yarn run build command from the repositories root directory. The deployable application lives in /platform/viewer/dist

When building the PWA application, you have the advantage of being able to specify the extensions you would like included at "build time" -- this takes place in platform/viewer/src/index.js.

Building and deploying the PWA application is not as clear cut, but you generally see much better performance. It allows us to leverage things like service workers, code splitting, treeshaking, improved minification, etc. that just aren't possible when trying to deliver the application as a single ES5 bundle of javascript.

That being said, this issue is still one I would like to see resolved, as it is a use case we would like to support, and will matter more as we progress to users being able to share "workflow recipes" (installable modes)

salimkanoun commented 4 years ago

Thanks for these information @dannyrb , I started to work on a React Project, but I'm very far from this level of complexity compared to OHIF code. Anyway I will try to make my own build of OHIF and see what's happening.

Best regards,

Salim

omer1616 commented 2 years ago

@dannyrb
hello, when I install other extensions except OHIFExtDicomMicroscopy to the project, I get this error and black screen comes up. can you help me please

error

sedghi commented 1 year ago

I'm gonna close this since a lot has changed. If the issue is relevant in v3 please re-open